php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #71270 Heap BufferOver Flow in escapeshell functions
Submitted: 2016-01-03 23:26 UTC Modified: 2016-01-21 11:46 UTC
From: emmanuel dot law at gmail dot com Assigned: ab
Status: Closed Package: Scripting Engine problem
PHP Version: 7.0.1 OS:
Private report: No CVE-ID: 2016-1904
 [2016-01-03 23:26 UTC] emmanuel dot law at gmail dot com
Description:
------------

1)I found this vulnerability using a custom PHP engine fuzzer that I wrote. There exist a heap-based buffer over flow that allows one to write a user tainted data pass an allocated buffer. This vulnerability lies in the following functions:
	escapeshellarg 
	escapeshellcmd 

2) On a default php installation, the memory limit is set to 128MB and this vulnerability is not triggerable. My analysis shows that this is triggerable when memory limit is roughly > 1024mb. A quick search on github shows that it's not uncommon to see code like "ini_set('memory_limit', -1);"


3)I've created a POC that triggers the buffer over write with 0x414141414141.....

4) A string of 1024mb is created and passed into escapeshellarg. "l" contains the length of this string:

Breakpoint 2, php_escape_shell_arg (str=0x7fffad469028 'A' <repeats 200 times>...) at /home/elaw/php-7.0.0/ext/standard/exec.c:343
343             int x, y = 0, l = (int)strlen(str);

gdb-peda$ print l
$43 = 0x40000000            // 1024mb



5) This length "l" is then passed into zend_string_alloc as "4 * l + 2" which results in an integer overflow:

Temporary breakpoint 3, php_escape_shell_arg (str=0x7fffad000018 'A' <repeats 200 times>...) at /home/elaw/php-7.0.1/ext/standard/exec.c:348
348             cmd = zend_string_alloc(4 * l + 2, 0); /* worst case */


gdb-peda$ print 4* l + 2
$44 = 0x2 				   //Overflow


6) Stepping into zend_string_alloc to verify the integer overflow. Notice len=0x2:
zend_string_alloc (persistent=0x0, len=0x2) at /home/elaw/php-7.0.0/Zend/zend_string.h:121      
121             zend_string *ret = (zend_string *)pemalloc(ZEND_MM_ALIGNED_SIZE(_ZSTR_STRUCT_SIZE(len)), persistent);


7) Lets confirm the overflow again in the allocated (zend_string *) cmd. Notice cmd.len=0x2:
gdb-peda$ p *cmd
$52 = {
  gc = {
    refcount = 0x1,
    u = {
      v = {
        type = 0x6,
        flags = 0x0,
        gc_info = 0x0
      },
      type_info = 0x6
    }
  },
  h = 0x0,
  len = 0x2,
  val = "1"
}



8) The loops then writes pass the allocated buffer in

258		for (x = 0, y = 0; x < l; x++) {
....
321       ZSTR_VAL(cmd)[y++] = str[x];



9) Verifying the buffer overflow in 
gdb-peda$ p (zend_string *)cmd.len
$9 = (zend_string *) 0x2
gdb-peda$ x/100b (zend_string *)cmd.val
0x1625a58:      0x27    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a60:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a68:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a70:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a78:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a80:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a88:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a90:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625a98:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625aa0:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625aa8:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625ab0:      0x41    0x41    0x41    0x41    0x41    0x41    0x41    0x41
0x1625ab8:      0x41    0x41    0x41    0x41


10) The vulnerability for php_escape_shell_cmd is identical.

11) I've created a POC for ubuntu x64 php7.
https://www.dropbox.com/s/og6g9bt1ulmibnd/escapeshellarg_Heap_BOF_POC.php?dl=0

11) I've also wrote a patch to uses zend_string_safe_alloc instead. For your consideration. 



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-01-03 23:29 UTC] emmanuel dot law at gmail dot com
Patch has been submitted as a pull request:
https://github.com/php/php-src/pull/1713
 [2016-01-04 18:02 UTC] ab@php.net
Thanks for the report, the issue and the fix confirmed. Please next time don't push a public PR but use a secret gist.

At this point, my question were yet - were it feasible to limit the length of the actual command line and/or argument to something appropriate. Fe using xargs --show-limits on Linux will give back a way smaller number than 1024M. On Windows it is indeed something measured in kilobytes.

Thanks.
 [2016-01-05 07:03 UTC] emmanuel dot law at gmail dot com
My bad.


Yeah I dont see why we cant harden it further by limiting the length of the command line and argument. It should however fail safe. EG if the length of string is beyond a certain limit, it should throw a fatal error rather than return a warning or an empty string.


For example, if it doesn't fail safe, the toy code below has the potential to delete all files on the webroot rather than only a user directory:

if(!ctype_alnum($_REQUEST[UID]))exit(); 
$path=escapeshellarg("/web/root/users/$_REQUEST[UID])/");
exec("cd $path ; rm *.*");
 [2016-01-05 11:30 UTC] ab@php.net
Yeah, that's a good example. It definitely should fail hard. In another bug #71039, I was poking about returning false for another case but it definitely seems a wrong way now.

However why the question about limiting length came into my mind is because now with your patch it will go through, but with a big string it will still be going too slow. Thus it still might be a matter of causing DOS. Fe on my relatively modern laptop it runs a couple of minutes to process the escapeshellarg() call.

Thanks.
 [2016-01-05 12:06 UTC] emmanuel dot law at gmail dot com
Checking the string length is a good additional control. However even if we don't, IMO I don't think that DOS is a big concern because of the default max_execution time (default 30 seconds)

EG the command below shows max_execution_time kicking in:

> time USE_ZEND_ALLOC=0 sapi/cli/php_patched_escapecmd_escapecmdarg -r 'set_time_limit(30); ini_set('memory_limit', -1); $A=str_repeat("A",1024*1024*1024);$A=escapeshellarg($A);'

Fatal error: Maximum execution time of 30 seconds exceeded in Command line code on line 1

real    0m30.940s
user    0m29.344s
sys     0m0.672s


It'll be hard to DOS the server if you need to post a 1024MB string to consume the PHP process for 30 seconds.
 [2016-01-05 13:18 UTC] ab@php.net
But that's not the extent. It also depends on the server configuration, of course. But in general, issue enough requests, even with less data that will keep the server busy, and here it is. At least that's what i had in mind.

Thanks.
 [2016-01-06 07:01 UTC] ab@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: ab
 [2016-01-06 07:01 UTC] ab@php.net
As the patch is already public, a decision was made to include it into 7.0.2 and do the other necessary follow-up work afterwards. The main issue is fixed, thanks for your work!
 [2016-01-21 11:46 UTC] kaplan@php.net
-CVE-ID: +CVE-ID: 2016-1904
 
PHP Copyright © 2001-2017 The PHP Group
All rights reserved.
Last updated: Tue Aug 29 15:01:52 2017 UTC