php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #74603 PHP INI Parsing Stack Buffer Overflow Vulnerability
Submitted: 2017-05-17 02:29 UTC Modified: 2017-08-17 15:41 UTC
From: l dot wei at ntu dot edu dot sg Assigned: stas
Status: Closed Package: Scripting Engine problem
PHP Version: 5.6.30 OS: *
Private report: No CVE-ID: 2017-11628
 [2017-05-17 02:29 UTC] l dot wei at ntu dot edu dot sg
Description:
------------
A stack buffer overflow exists in the latest stable release of PHP-7.1.5 and PHP-5.6.30 in PHP INI parsing API, which may accept network / local filesystem input. On malformed inputs, a stack buffer overflow in zend_ini_do_op() could write 1-byte off a fixed size stack buffer. On installations with the stack smashing mitigation, this would cause an immediate DoS; upto optimization levels, build options and stack buffer overflow mitigations, this vulnerability may allow corrupting other local variables or the frame pointer, potentially allows remotely executing code.

In php-7.1.5/Zend/zend_long.h:

110 #if SIZEOF_ZEND_LONG == 4
111 # define MAX_LENGTH_OF_LONG 11
112 # define LONG_MIN_DIGITS "2147483648"
113 #elif SIZEOF_ZEND_LONG == 8
114 # define MAX_LENGTH_OF_LONG 20
115 # define LONG_MIN_DIGITS "9223372036854775808"
116 #else
117 # error "Unknown SIZEOF_ZEND_LONG"
118 #endif

In php-7.1.5/Zend/zend_ini_parser.c:

 123 /* {{{ zend_ini_do_op()
 124 */
 125 static void zend_ini_do_op(char type, zval *result, zval *op1, zval *op2)
 126 {
 127         int i_result;
 128         int i_op1, i_op2;
 129         int str_len;
 130         char str_result[MAX_LENGTH_OF_LONG];
 131 
 132         i_op1 = atoi(Z_STRVAL_P(op1));
 133         zend_string_free(Z_STR_P(op1));
 134         if (op2) {
 135                 i_op2 = atoi(Z_STRVAL_P(op2));
 136                 zend_string_free(Z_STR_P(op2));
 137         } else {
 138                 i_op2 = 0;
 139         }
 140 
 141         switch (type) {
 142                 case '|':
 143                         i_result = i_op1 | i_op2;
 144                         break;
 145                 case '&':
 146                         i_result = i_op1 & i_op2;
 147                         break;
 148                 case '^':
 149                         i_result = i_op1 ^ i_op2;
 150                         break;
 151                 case '~':
 152                         i_result = ~i_op1;
 153                         break;
 154                 case '!':
 155                         i_result = !i_op1;
 156                         break;
 157                 default:
 158                         i_result = 0;
 159                         break;
 160         }
 161 
 162         str_len = zend_sprintf(str_result, "%d", i_result);
 163         ZVAL_NEW_STR(result, zend_string_init(str_result, str_len, ZEND_SYSTEM_INI));
 164 }
 165 /* }}} */

The minimums, "-2147483648" and "-9223372036854775808" are of length 11 and 20 respectively, 
the proper definition of str_result[] array would be: str_result[MAX_LENGTH_OF_LONG + 1]. A
crafted ini entry would cause an overflow.

Credit: Wei Lei and Liu Yang of Nanyang Technological University.

Test script:
---------------
$ cat input.ini
0=0&~2000000000

$ cat input.php
<?php 

$argc = $_SERVER['argc'];
$argv = $_SERVER['argv'];

$file_loc = dirname(__FILE__)."/".$argv[1];

var_dump(parse_ini_file($file_loc, true, INI_SCANNER_NORMAL));

?>

Expected result:
----------------
No crash.

Actual result:
--------------
$ bin/php input.php input.ini

*** buffer overflow detected ***: bin/php terminated
======= Backtrace: =========
/lib/i386-linux-gnu/libc.so.6(+0x68e4e)[0xb7527e4e]
/lib/i386-linux-gnu/libc.so.6(__fortify_fail+0x6b)[0xb75ba85b]
/lib/i386-linux-gnu/libc.so.6(+0xfa6ea)[0xb75b96ea]
/lib/i386-linux-gnu/libc.so.6(+0xf9e48)[0xb75b8e48]
/lib/i386-linux-gnu/libc.so.6(_IO_default_xsputn+0x8e)[0xb752fc0e]
/lib/i386-linux-gnu/libc.so.6(_IO_vfprintf+0x89b)[0xb7502f3b]
/lib/i386-linux-gnu/libc.so.6(__vsprintf_chk+0xb1)[0xb75b8f01]
/lib/i386-linux-gnu/libc.so.6(__sprintf_chk+0x2f)[0xb75b8e2f]
bin/php[0x82e7aa0]
bin/php[0x82e87d3]
bin/php(zend_parse_ini_file+0x47)[0x82e8b07]
bin/php[0x8255788]
bin/php[0x83631f6]
bin/php(execute_ex+0x22)[0x8353c52]
bin/php(zend_execute+0x13b)[0x83a341b]
bin/php(zend_execute_scripts+0x30)[0x8313010]
bin/php(php_execute_script+0x286)[0x82b3f26]
bin/php[0x83a57de]
bin/php[0x80683b9]
/lib/i386-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0xb74d8a83]
bin/php[0x8068444]
======= Memory map: ========
08048000-0888d000 r-xp 00000000 08:01 704181     /home/weilei/php7_gdb/bin/php
0888d000-0888e000 r--p 00844000 08:01 704181     /home/weilei/php7_gdb/bin/php
0888e000-08899000 rw-p 00845000 08:01 704181     /home/weilei/php7_gdb/bin/php
08899000-088b2000 rw-p 00000000 00:00 0 
09ae7000-09b9a000 rw-p 00000000 00:00 0          [heap]
b7000000-b7200000 r--p 00000000 08:01 271314     /usr/lib/locale/locale-archive
b7200000-b7400000 rw-p 00000000 00:00 0 
b7464000-b7480000 r-xp 00000000 08:01 787579     /lib/i386-linux-gnu/libgcc_s.so.1
b7480000-b7481000 rw-p 0001b000 08:01 787579     /lib/i386-linux-gnu/libgcc_s.so.1
b7496000-b74bf000 rw-p 00000000 00:00 0 
b74bf000-b7667000 r-xp 00000000 08:01 787552     /lib/i386-linux-gnu/libc-2.19.so
b7667000-b7669000 r--p 001a8000 08:01 787552     /lib/i386-linux-gnu/libc-2.19.so
b7669000-b766a000 rw-p 001aa000 08:01 787552     /lib/i386-linux-gnu/libc-2.19.so
b766a000-b766d000 rw-p 00000000 00:00 0 
b766d000-b7670000 r-xp 00000000 08:01 787569     /lib/i386-linux-gnu/libdl-2.19.so
b7670000-b7671000 r--p 00002000 08:01 787569     /lib/i386-linux-gnu/libdl-2.19.so
b7671000-b7672000 rw-p 00003000 08:01 787569     /lib/i386-linux-gnu/libdl-2.19.so
b7672000-b7673000 rw-p 00000000 00:00 0 
b7673000-b76b7000 r-xp 00000000 08:01 787602     /lib/i386-linux-gnu/libm-2.19.so
b76b7000-b76b8000 r--p 00043000 08:01 787602     /lib/i386-linux-gnu/libm-2.19.so
b76b8000-b76b9000 rw-p 00044000 08:01 787602     /lib/i386-linux-gnu/libm-2.19.so
b76b9000-b76cc000 r-xp 00000000 08:01 787678     /lib/i386-linux-gnu/libresolv-2.19.so
b76cc000-b76cd000 ---p 00013000 08:01 787678     /lib/i386-linux-gnu/libresolv-2.19.so
b76cd000-b76ce000 r--p 00013000 08:01 787678     /lib/i386-linux-gnu/libresolv-2.19.so
b76ce000-b76cf000 rw-p 00014000 08:01 787678     /lib/i386-linux-gnu/libresolv-2.19.so
b76cf000-b76e4000 rw-p 00000000 00:00 0 
b76e4000-b76e5000 r--s 00000000 08:01 554280     /home/weilei/php7_gdb/input.ini
b76e5000-b76e6000 r--p 00855000 08:01 271314     /usr/lib/locale/locale-archive
b76e6000-b76e8000 rw-p 00000000 00:00 0 
b76e8000-b76ea000 r--p 00000000 00:00 0          [vvar]
b76ea000-b76ec000 r-xp 00000000 00:00 0          [vdso]
b76ec000-b770c000 r-xp 00000000 08:01 787528     /lib/i386-linux-gnu/ld-2.19.so
b770c000-b770d000 r--p 0001f000 08:01 787528     /lib/i386-linux-gnu/ld-2.19.so
b770d000-b770e000 rw-p 00020000 08:01 787528     /lib/i386-linux-gnu/ld-2.19.so
bff25000-bff47000 rw-p 00000000 00:00 0          [stack]
Aborted



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2017-05-28 08:44 UTC] l dot wei at ntu dot edu dot sg
-Package: Reproducible crash +Package: Scripting Engine problem
 [2017-05-28 08:44 UTC] l dot wei at ntu dot edu dot sg
Affecting Zend, changed package to Scripting Engine. Can also be triggered via ini string (e.g., in cmdline argument, use -d foor=[bar] with the crasher above).
 [2017-06-19 23:39 UTC] stas@php.net
-Type: Security +Type: Bug
 [2017-06-19 23:39 UTC] stas@php.net
Since .ini files are inherently admin functions, not a security issue.
 [2017-06-20 04:39 UTC] l dot wei at ntu dot edu dot sg
Hi stas, thanks for the reply.

It is true that php.ini should be protected from tampering, if modified, other issues could arise too.

However it seems to me that parse_ini_file() and parse_ini_string() are not limited to protected ini files, as there is no inherent restriction whatsoever that stops these functions from being called on untrusted external inputs, especially for parse_ini_string(), as a handy PHP API.

So I think maybe this should qualify a security bug ? The implementation bears some assumptions that ini files/strings should always be protected to the same level as the PHP module, however such implicit requirements cannot be guaranteed.
 [2017-06-20 05:07 UTC] stas@php.net
Seems to be reproducible only on 32-bit PHP, since i_op* are ints, so it is required that sizeof(int) == sizeof(long) to reproduce the issue.
 [2017-06-20 05:50 UTC] l dot wei at ntu dot edu dot sg
-Type: Bug +Type: Security -PHP Version: 7.1.5 +PHP Version: 7.1.6 -Private report: No +Private report: Yes
 [2017-06-20 05:50 UTC] l dot wei at ntu dot edu dot sg
Was tested only on 32-bit, seems not reproducing for 64-bit. So affecting both 5 and 7 on 32-bit builds.
 [2017-06-20 07:13 UTC] stas@php.net
-PHP Version: 7.1.6 +PHP Version: 5.6.30 -Assigned To: +Assigned To: stas
 [2017-06-20 07:13 UTC] stas@php.net
The fix is in security repo as fec9a2e1b599b870c22733047d16cb1fa18ca711 and in https://gist.github.com/5187488335eff73b816f0d4ed0d4290f

Please verify
 [2017-06-20 08:27 UTC] l dot wei at ntu dot edu dot sg
Hi stas, the fix looks good, thanks!
 [2017-07-05 04:13 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=5f8380d33e648964d2d5140f329cf2d4c443033c
Log: Fix bug #74603 - use correct buffer size
 [2017-07-05 04:13 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2017-07-05 04:23 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=05255749139b3686c8a6a58ee01131ac0047465e
Log: Fix bug #74603 - use correct buffer size
 [2017-07-05 04:23 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=5f8380d33e648964d2d5140f329cf2d4c443033c
Log: Fix bug #74603 - use correct buffer size
 [2017-07-05 05:20 UTC] l dot wei at ntu dot edu dot sg
Hi, does this issue get a CVE id ?
 [2017-07-05 06:13 UTC] remi@php.net
This bug requires some specially crafted ini file, so IMHO is not even a security issue.
 [2017-07-05 06:53 UTC] l dot wei at ntu dot edu dot sg
Not all ini files receive the same protection as php.ini (if properly configured). That allows a security boundary cross. It is provided by PHP as an API, together with parse_ini_string() - which means there is no restriction on what to be passed into.
 [2017-07-05 16:25 UTC] stas@php.net
-CVE-ID: +CVE-ID: needed
 [2017-07-06 06:44 UTC] krakjoe@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=88c521d659521f695e0b9e7fcdded0fc1ee1c53b
Log: Fix bug #74603 - use correct buffer size
 [2017-07-25 08:50 UTC] chiuado at gmail dot com
Sorry for bothering, is this issue reproducible on windows platform?
I'm using "php-5.6.30-Win32-VC11-x86", but I can not reproduce it, is there any precondition?
 [2017-07-25 11:21 UTC] l dot wei at ntu dot edu dot sg
static void zend_ini_do_op(char type, zval *result, zval *op1, zval *op2)
{
	int i_result;
	int i_op1, i_op2;
	char str_result[MAX_LENGTH_OF_LONG];

On the mentioned Windows build, checking the following two:

bp !php5ts+436f0     // ini_parse() 
bp !php5ts+375d20    // zend_ini_do_op()

Stack cookie protection is on, with canary store in [ebp-4]. The mitigation re-ordered the 4-byte aligned char array str_result to be next to the canary, starting at [ebp-10h]. When the overflow occurred, the 1-byte '\0' just occupied the 1-byte padding. Therefore it does not affect this build.

When mitigation is not in place, or the string is not aligned to 4-byte, it may cause an immediate DoS (cookie corruption); or 1-byte of ebp (no stack smash protection).
 [2017-07-26 01:36 UTC] l dot wei at ntu dot edu dot sg
CVE-2017-11628 is assigned for this issue.
 [2017-07-26 01:44 UTC] chiuado at gmail dot com
Understood, thanks for the clearly explanation!
 [2017-07-30 20:59 UTC] stas@php.net
-CVE-ID: needed +CVE-ID: 2017-11628
 [2017-08-17 09:18 UTC] zeev@php.net
-Type: Security +Type: Bug
 [2017-08-17 09:18 UTC] zeev@php.net
I'm changing this back to 'Bug'.

When coming to determine whether something is a security issue or not, we have to consider the likelihood of a real world attack vector.  In here, it's low to non-existent - since even in apps that use parse_ini_file() - they do that for their own configuration.  If you're an app admin trying to 'attack' your own deployment, be our guest and succeed.  And if you're a user that installs an app from an untrusted source (that may come bundled with a malicious .ini file) - you have bigger issues than this buffer overflow - as an app from an untrusted source could already do pretty much everything the user credentials allow.  Loading your configuration from a remote untrusted source?  I just don't see that happening.

The only scenario I can think of this can be a security issue, is an app that manages or analyzes uploaded/remote .ini files.  That is such a narrow/theoretical use case I don't think we should pay attention to it.

Perhaps we should add a new 'Security (very minor)' category, or something of the sort - but these sorts of bugs can't be in the same category as, say, a remote vulnerability.
 [2017-08-17 15:41 UTC] l dot wei at ntu dot edu dot sg
Normally we audit functions that take external input, as they involve assumptions that could be challenged under certain settings, or the specification. As we see it, this issue does open additional possibilities than just accepting an ini setting or rejecting it. An unprotected ini vector, if requiring lower privilege, could be used to chain with other issues and cross the boundary. You are right this is just theoretical. 

At least I don't see how this is more theoretical than those full bunch of integer overflows triggered by scripting arguments. Blame the inconsistency of your policy handling such issues. We work for free too.
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Tue Aug 29 15:01:52 2017 UTC