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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: ab@php.net
New email:
PHP Version: OS:

 

 [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: Thu Nov 21 11:01:29 2024 UTC