php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #69646 OS command injection vulnerability in escapeshellarg
Submitted: 2015-05-15 23:12 UTC Modified: 2015-06-18 12:31 UTC
From: ab@php.net Assigned: ab (profile)
Status: Closed Package: Program Execution
PHP Version: Irrelevant OS: Windows
Private report: No CVE-ID: 2015-4642
 [2015-05-15 23:12 UTC] ab@php.net
Description:
------------
In following is the report from Takayuki Uchiyama.

This issue is an OS command injection vulnerability.

Do you have a specific case that fails?

I have attached the proof-of-concept code to reproduce this issue.

----------------------------------------------------------------------
PoC Code
----------------------------------------------------------------------
    [poc.php]
    ------------------
    <?php
      $a = 'a\\';
      $b = 'b -c d\\';
      var_dump( $a, escapeshellarg($a) );
      var_dump( $b, escapeshellarg($b) );
      system( 'php arginfo.php ' . escapeshellarg($a) . ' ' . escapeshellarg($b) )
    ?>
    ------------------

    [arginfo.php]
    ------------------
    <?php
      print( "--- ARG INFO ---\n" );
      var_dump( $argv );
    ?>
    ------------------
----------------------------------------------------------------------
PoC Code
----------------------------------------------------------------------


After running 'php poc.php', if you get the following output, that version
of PHP is still vulnerable.

----------------------------------------------------------------------
Output
----------------------------------------------------------------------
string(2) "a\"
string(4) ""a\""
string(7) "b -c d\"
string(9) ""b -c d\""
--- ARG INFO ---
array(4) {
 [0]=>
 string(11) "arginfo.php"
 [1]=>
 string(4) "a" b"
 [2]=>
 string(2) "-c"
 [3]=>
  string(2) "d""
}

[Comment]
The first 4 lines are the output from the var_dump function in 
poc.php. By comparing this output with the 4-5th lines of poc.php, 
the output from the escapeshellarg function, it can be seen that
an attacker can set a single string that is not "" escaped as a
parameter.

Similarly, the 10 lines that follow --- ARG INFO --- command line 
arguments when arginfo.php is called, which are output by the var_dump 
function in arginfo.php. When comparing this to the way the system 
function is called (with 2 parameters) in poc.php, it can be seen that 
command line interprets is as 3 paramaters.
----------------------------------------------------------------------
Output
----------------------------------------------------------------------



Patches

bug69464_php7.patch (last revision 2015-05-18 14:49 UTC by ab@php.net)
bug64646_php5_plus_test (last revision 2015-05-18 14:48 UTC by ab@php.net)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2015-05-15 23:13 UTC] ab@php.net
-Status: Open +Status: Verified -Assigned To: +Assigned To: ab
 [2015-05-15 23:13 UTC] ab@php.net
I can reproduce this with 5.6 and master, probably need a patch for lower as well.
 [2015-05-18 14:48 UTC] ab@php.net
The following patch has been added/updated:

Patch Name: bug64646_php5_plus_test
Revision:   1431960482
URL:        https://bugs.php.net/patch-display.php?bug=69646&patch=bug64646_php5_plus_test&revision=1431960482
 [2015-05-18 14:49 UTC] ab@php.net
The following patch has been added/updated:

Patch Name: bug69464_php7.patch
Revision:   1431960565
URL:        https://bugs.php.net/patch-display.php?bug=69646&patch=bug69464_php7.patch&revision=1431960565
 [2015-05-18 14:51 UTC] ab@php.net
Regarding the patches - when merging up from php5, the php7 variant should be used. Or, in the php5 to php7 conflict, the only difference is the usage of zend_string, so it might be even corrected just while resolving the conflict.
 [2015-06-10 04:40 UTC] stas@php.net
-Status: Verified +Status: Closed
 [2015-06-10 04:40 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2015-06-10 07:42 UTC] tyrael@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=d2ac264ffea5ca2e85640b6736e0c7cd4ee9a4a9
Log: Fix bug #69646	OS command injection vulnerability in escapeshellarg
 [2015-06-10 08:50 UTC] tyrael@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=563462fbf8c45c5fe54c6b215d6ec76b5b3f867c
Log: Fixed bug #69646 (OS command injection vulnerability in escapeshellarg)
 [2015-06-10 08:50 UTC] tyrael@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=80367584910885baa1a2a4476a4a31efdcf0c9c0
Log: Fix bug #69646	OS command injection vulnerability in escapeshellarg
 [2015-06-10 09:15 UTC] jpauli@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=e22f22e6ae83298454576f80b1a91373e7a21ce2
Log: Fix bug #69646	OS command injection vulnerability in escapeshellarg
 [2015-06-11 12:29 UTC] php at bof dot de
I failed to reproduce the issue under Linux, and wondered that I saw, contrary to the shown output, that PHP uses single quotes instead of double quotes under Linux.

After some mail exchange with ab@php.net it turns out this issue affects Windows only.

Just noting that for clarification, should somebody wonder, too...
 [2015-06-18 12:31 UTC] kaplan@php.net
-CVE-ID: +CVE-ID: 2015-4642
 [2015-07-01 17:39 UTC] francois dot gagne at gmail dot com
Should the buffer allocation account for this extra \ ?

PHP 5.5:

   cmd = safe_emalloc(4, l, 3); /* worst case */

In 5.5, the +3 is for the starting and ending " and the NUL byte.


PHP master:

   cmd = zend_string_alloc(4 * l + 2, 0); /* worst case */

In master, the +2 is for the starting and ending '.


This is not exploitable since to add an extra \ the string must ends with a \ which is a 1 byte wide character and each characters is allocated 4 bytes.

I still think this would make it clearer and more future proof. Perhaps add a comment about the allocation calculation?
 [2016-07-20 11:38 UTC] davey@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=563462fbf8c45c5fe54c6b215d6ec76b5b3f867c
Log: Fixed bug #69646 (OS command injection vulnerability in escapeshellarg)
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Nov 23 07:01:29 2024 UTC