On Thu, Nov 10, 2011 at 3:19 PM, Rui Hirokawa <hirok...@php.net> wrote:

> hirokawa                                 Thu, 10 Nov 2011 14:19:06 +0000
>
> Revision: http://svn.php.net/viewvc?view=revision&revision=318996
>
> Log:
> MFH: fixed bug #60116 (escapeshellcmd() cannot escape the characters which
> cause shell command injection).
>
> Bug: https://bugs.php.net/60116 (error getting bug information)
>
> Changed paths:
>    U   php/php-src/branches/PHP_5_4/NEWS
>    U   php/php-src/branches/PHP_5_4/ext/standard/exec.c
>    A
> php/php-src/branches/PHP_5_4/ext/standard/tests/general_functions/bug60116.phpt
>
> Modified: php/php-src/branches/PHP_5_4/NEWS
> ===================================================================
> --- php/php-src/branches/PHP_5_4/NEWS   2011-11-10 14:12:48 UTC (rev
> 318995)
> +++ php/php-src/branches/PHP_5_4/NEWS   2011-11-10 14:19:06 UTC (rev
> 318996)
> @@ -24,8 +24,10 @@
>   . Fixed bug #60169 (Conjunction of ternary and list crashes PHP).
>     (Laruence)
>   . Fixed bug #55475 (is_a() triggers autoloader, new optional 3rd
> argument to
> -    is_a and is_subclass_of). (alan_k)
> -
> +    is_a and is_subclass_of). (alan_k)
> +  . Fixed bug #60116 (escapeshellcmd() cannot escape the characters
> +     which cause shell command injection). (rui)
> +
>  - Oracle Database extension (OCI8):
>   . Increased maxium Oracle error message buffer length for new 11.2.0.3
> size
>     (Chris Jones)
>
> Modified: php/php-src/branches/PHP_5_4/ext/standard/exec.c
> ===================================================================
> --- php/php-src/branches/PHP_5_4/ext/standard/exec.c    2011-11-10
> 14:12:48 UTC (rev 318995)
> +++ php/php-src/branches/PHP_5_4/ext/standard/exec.c    2011-11-10
> 14:19:06 UTC (rev 318996)
> @@ -50,6 +50,16 @@
>  #include <unistd.h>
>  #endif
>
> +/* {{{ register_exec_constants
> + *  */
> +void register_exec_constants(INIT_FUNC_ARGS)
> +{
> +    REGISTER_LONG_CONSTANT("ESCAPE_CMD_PAIR", ESCAPE_CMD_PAIR,
> CONST_PERSISTENT|CONST_CS);
> +    REGISTER_LONG_CONSTANT("ESCAPE_CMD_END", ESCAPE_CMD_END,
> CONST_PERSISTENT|CONST_CS);
> +    REGISTER_LONG_CONSTANT("ESCAPE_CMD_ALL", ESCAPE_CMD_ALL,
> CONST_PERSISTENT|CONST_CS);
> +}
> +/* }}} */
> +
>  /* {{{ php_exec
>  * If type==0, only last line of output is returned (exec)
>  * If type==1, all lines will be printed and last lined returned (system)
> @@ -238,7 +248,7 @@
>
>    *NOT* safe for binary strings
>  */
> -PHPAPI char *php_escape_shell_cmd(char *str)
> +PHPAPI char *php_escape_shell_cmd_ex(char *str, int flag)
>  {
>        register int x, y, l = strlen(str);
>        char *cmd;
> @@ -266,14 +276,26 @@
>  #ifndef PHP_WIN32
>                        case '"':
>                        case '\'':
> -                               if (!p && (p = memchr(str + x + 1, str[x],
> l - x - 1))) {
> -                                       /* noop */
> -                               } else if (p && *p == str[x]) {
> -                                       p = NULL;
> -                               } else {
> +                               if (flag == ESCAPE_CMD_ALL) {
>                                        cmd[y++] = '\\';
> +                                       cmd[y++] = str[x];
> +                               } else if (flag == ESCAPE_CMD_END) {
> +                                       if ((x == 0 || x == l - 1) &&
> (str[0] == str[l-1])) {
> +                                               cmd[y++] = str[x];
> +                    } else {
> +                        cmd[y++] = '\\';
> +                        cmd[y++] = str[x];
> +                    }
> +                               } else { /* ESCAPE_CMD_PAIR */
> +                                       if (!p && (p = memchr(str + x + 1,
> str[x], l - x - 1))) {
> +                                               /* noop */
> +                                       } else if (p && *p == str[x]) {
> +                                               p = NULL;
> +                                       } else {
> +                                               cmd[y++] = '\\';
> +                                       }
> +                                       cmd[y++] = str[x];
>                                }
> -                               cmd[y++] = str[x];
>                                break;
>  #else
>                        /* % is Windows specific for enviromental
> variables, ^%PATH% will
> @@ -327,6 +349,14 @@
>  }
>  /* }}} */
>
> +/* {{{ php_escape_shell_cmd
> + */
> +PHPAPI char *php_escape_shell_cmd(char *str)
> +{
> +    return php_escape_shell_cmd_ex(str, ESCAPE_CMD_PAIR);
> +}
> +/* }}} */
> +
>  /* {{{ php_escape_shell_arg
>  */
>  PHPAPI char *php_escape_shell_arg(char *str)
> @@ -397,14 +427,15 @@
>  {
>        char *command;
>        int command_len;
> +       long flag = ESCAPE_CMD_PAIR;
>        char *cmd = NULL;
>
> -       if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s",
> &command, &command_len) == FAILURE) {
> +       if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s|l",
> &command, &command_len, &flag) == FAILURE) {
>                return;
>        }
>
>        if (command_len) {
> -               cmd = php_escape_shell_cmd(command);
> +               cmd = php_escape_shell_cmd_ex(command, flag);
>                RETVAL_STRING(cmd, 0);
>        } else {
>                RETVAL_EMPTY_STRING();
>
> Added:
> php/php-src/branches/PHP_5_4/ext/standard/tests/general_functions/bug60116.phpt
> ===================================================================
> ---
> php/php-src/branches/PHP_5_4/ext/standard/tests/general_functions/bug60116.phpt
>                             (rev 0)
> +++
> php/php-src/branches/PHP_5_4/ext/standard/tests/general_functions/bug60116.phpt
>     2011-11-10 14:19:06 UTC (rev 318996)
> @@ -0,0 +1,160 @@
> +--TEST--
> +Test escapeshellcmd() to escape the quotation
> +--SKIPIF--
> +<?php
> +if( substr(PHP_OS, 0, 3) == 'WIN' ) {
> +   die('skip...Invalid for Windows');
> +}
> +?>
> +--FILE--
> +<?php
> +echo "*** Testing escapeshellcmd() escape the quotation ***\n";
> +$data = array(
> +       '"abc',
> +       "'abc",
> +       '?<>',
> +       '()[]{}$',
> +       '%^',
> +       '#&;`|*?',
> +       '~<>\\',
> +       '%NOENV%',
> +       "abc' 'def",
> +       'abc" "def',
> +       "'abc def'",
> +       '"abc def"',
> +);
> +
> +echo "case: default\n";
> +
> +$count = 1;
> +foreach ($data AS $value) {
> +       echo "-- Test " . $count++ . " --\n";
> +       var_dump(escapeshellcmd($value));
> +}
> +
> +echo "case: ESCAPE_CMD_PAIR\n";
> +$count = 1;
> +foreach ($data AS $value) {
> +       echo "-- Test " . $count++ . " --\n";
> +       var_dump(escapeshellcmd($value, ESCAPE_CMD_PAIR));
> +}
> +
> +echo "case: ESCAPE_CMD_END\n";
> +$count = 1;
> +foreach ($data AS $value) {
> +       echo "-- Test " . $count++ . " --\n";
> +       var_dump(escapeshellcmd($value, ESCAPE_CMD_END));
> +}
> +
> +echo "case: ESCAPE_CMD_ALL\n";
> +$count = 1;
> +foreach ($data AS $value) {
> +       echo "-- Test " . $count++ . " --\n";
> +       var_dump(escapeshellcmd($value, ESCAPE_CMD_ALL));
> +}
> +
> +echo "Done\n";
> +?>
> +--EXPECTF--
> +*** Testing escapeshellcmd() escape the quotation ***
> +case: default
> +-- Test 1 --
> +string(5) "\"abc"
> +-- Test 2 --
> +string(5) "\'abc"
> +-- Test 3 --
> +string(6) "\?\<\>"
> +-- Test 4 --
> +string(14) "\(\)\[\]\{\}\$"
> +-- Test 5 --
> +string(3) "%\^"
> +-- Test 6 --
> +string(14) "\#\&\;\`\|\*\?"
> +-- Test 7 --
> +string(8) "\~\<\>\\"
> +-- Test 8 --
> +string(7) "%NOENV%"
> +-- Test 9 --
> +string(9) "abc' 'def"
> +-- Test 10 --
> +string(9) "abc" "def"
> +-- Test 11 --
> +string(9) "'abc def'"
> +-- Test 12 --
> +string(9) ""abc def""
> +case: ESCAPE_CMD_PAIR
> +-- Test 1 --
> +string(5) "\"abc"
> +-- Test 2 --
> +string(5) "\'abc"
> +-- Test 3 --
> +string(6) "\?\<\>"
> +-- Test 4 --
> +string(14) "\(\)\[\]\{\}\$"
> +-- Test 5 --
> +string(3) "%\^"
> +-- Test 6 --
> +string(14) "\#\&\;\`\|\*\?"
> +-- Test 7 --
> +string(8) "\~\<\>\\"
> +-- Test 8 --
> +string(7) "%NOENV%"
> +-- Test 9 --
> +string(9) "abc' 'def"
> +-- Test 10 --
> +string(9) "abc" "def"
> +-- Test 11 --
> +string(9) "'abc def'"
> +-- Test 12 --
> +string(9) ""abc def""
> +case: ESCAPE_CMD_END
> +-- Test 1 --
> +string(5) "\"abc"
> +-- Test 2 --
> +string(5) "\'abc"
> +-- Test 3 --
> +string(6) "\?\<\>"
> +-- Test 4 --
> +string(14) "\(\)\[\]\{\}\$"
> +-- Test 5 --
> +string(3) "%\^"
> +-- Test 6 --
> +string(14) "\#\&\;\`\|\*\?"
> +-- Test 7 --
> +string(8) "\~\<\>\\"
> +-- Test 8 --
> +string(7) "%NOENV%"
> +-- Test 9 --
> +string(11) "abc\' \'def"
> +-- Test 10 --
> +string(11) "abc\" \"def"
> +-- Test 11 --
> +string(9) "'abc def'"
> +-- Test 12 --
> +string(9) ""abc def""
> +case: ESCAPE_CMD_ALL
> +-- Test 1 --
> +string(5) "\"abc"
> +-- Test 2 --
> +string(5) "\'abc"
> +-- Test 3 --
> +string(6) "\?\<\>"
> +-- Test 4 --
> +string(14) "\(\)\[\]\{\}\$"
> +-- Test 5 --
> +string(3) "%\^"
> +-- Test 6 --
> +string(14) "\#\&\;\`\|\*\?"
> +-- Test 7 --
> +string(8) "\~\<\>\\"
> +-- Test 8 --
> +string(7) "%NOENV%"
> +-- Test 9 --
> +string(11) "abc\' \'def"
> +-- Test 10 --
> +string(11) "abc\" \"def"
> +-- Test 11 --
> +string(11) "\'abc def\'"
> +-- Test 12 --
> +string(11) "\"abc def\""
> +Done
>
>
> Property changes on:
> php/php-src/branches/PHP_5_4/ext/standard/tests/general_functions/bug60116.phpt
> ___________________________________________________________________
> Added: svn:keywords
>   + Id Rev Revision
> Added: svn:eol-style
>   + native
>
>
> --
> PHP CVS Mailing List (http://www.php.net/)
> To unsubscribe, visit: http://www.php.net/unsub.php
>


this change break the build
http://ci.qa.php.net/job/php-src-5.4-matrix-build/127/
could you look into it?

*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:
In function ‘register_exec_constants’:*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:57:
error: ‘ESCAPE_CMD_PAIR’ undeclared (first use in this
function)*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:57:
error: (Each undeclared identifier is reported only once*14:23:27*
  [exec] 
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:57:
error: for each function it appears in.)*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:58:
error: ‘ESCAPE_CMD_END’ undeclared (first use in this
function)*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:59:
error: ‘ESCAPE_CMD_ALL’ undeclared (first use in this
function)*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:
In function ‘php_escape_shell_cmd_ex’:*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:279:
error: ‘ESCAPE_CMD_ALL’ undeclared (first use in this
function)*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:282:
error: ‘ESCAPE_CMD_END’ undeclared (first use in this
function)*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:
In function ‘php_escape_shell_cmd’:*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:356:
error: ‘ESCAPE_CMD_PAIR’ undeclared (first use in this
function)*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:
In function ‘zif_escapeshellcmd’:*14:23:27*       [exec]
/home/jenkins/workspace/php-src-5.4-matrix-build/architecture/x86/os/linux-debian-6.0/ext/standard/exec.c:430:
error: ‘ESCAPE_CMD_PAIR’ undeclared (first use in this
function)*14:23:27*       [exec] make: *** [ext/standard/exec.lo]
Error 1*14:23:27*


-- 
Ferenc Kovács
@Tyr43l - http://tyrael.hu

Reply via email to