From 7e9abe142121f4331102d76aecff3ee5ac52b09d Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 24 Jun 2019 12:53:40 +0200 Subject: [PATCH 1/6] Add support for proc_open() with a command array (Unix) In this case the program will be executed directly, without a shell. --- ext/standard/proc_open.c | 89 ++++++++++++++++--- .../general_functions/proc_open_array.phpt | 30 +++++++ 2 files changed, 105 insertions(+), 14 deletions(-) create mode 100644 ext/standard/tests/general_functions/proc_open_array.phpt diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index d76e1595f3049..d5bbd124cd82d 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -403,8 +403,9 @@ struct php_proc_open_descriptor_item { Run a process with more control over it's file descriptors */ PHP_FUNCTION(proc_open) { + zval *command_zv; char *command, *cwd=NULL; - size_t command_len, cwd_len = 0; + size_t cwd_len = 0; zval *descriptorspec; zval *pipes; zval *environment = NULL; @@ -428,23 +429,23 @@ PHP_FUNCTION(proc_open) char cur_cwd[MAXPATHLEN]; wchar_t *cmdw = NULL, *cwdw = NULL, *envpw = NULL; size_t tmp_len; -#endif - php_process_id_t child; - struct php_process_handle *proc; - int is_persistent = 0; /* TODO: ensure that persistent procs will work */ -#ifdef PHP_WIN32 int suppress_errors = 0; int bypass_shell = 0; int blocking_pipes = 0; int create_process_group = 0; +#else + char **argv = NULL; #endif + php_process_id_t child; + struct php_process_handle *proc; + int is_persistent = 0; /* TODO: ensure that persistent procs will work */ #if PHP_CAN_DO_PTS php_file_descriptor_t dev_ptmx = -1; /* master */ php_file_descriptor_t slave_pty = -1; #endif ZEND_PARSE_PARAMETERS_START(3, 6) - Z_PARAM_STRING(command, command_len) + Z_PARAM_ZVAL(command_zv) Z_PARAM_ARRAY(descriptorspec) Z_PARAM_ZVAL(pipes) Z_PARAM_OPTIONAL @@ -453,7 +454,43 @@ PHP_FUNCTION(proc_open) Z_PARAM_ARRAY_EX(other_options, 1, 0) ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE); - command = pestrdup(command, is_persistent); + if (Z_TYPE_P(command_zv) == IS_ARRAY) { + zval *arg_zv; + uint32_t num_elems = zend_hash_num_elements(Z_ARRVAL_P(command_zv)); + if (num_elems == 0) { + php_error_docref(NULL, E_WARNING, "Command array must have at least one element"); + RETURN_FALSE; + } + +#ifdef PHP_WIN32 + TODO +#else + argv = safe_emalloc(sizeof(char *), num_elems + 1, 0); + i = 0; + ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(command_zv), arg_zv) { + zend_string *arg_str = zval_try_get_string(arg_zv); + if (!arg_str) { + int j; + for (j = 0; j < i; j++) { + efree(argv[j]); + } + efree(argv); + return; + } + + if (i == 0) { + command = pestrdup(ZSTR_VAL(arg_str), is_persistent); + } + + argv[i++] = estrdup(ZSTR_VAL(arg_str)); + zend_string_release(arg_str); + } ZEND_HASH_FOREACH_END(); + argv[i] = NULL; +#endif + } else { + convert_to_string(command_zv); + command = pestrdup(Z_STRVAL_P(command_zv), is_persistent); + } #ifdef PHP_WIN32 if (other_options) { @@ -487,8 +524,6 @@ PHP_FUNCTION(proc_open) } #endif - command_len = strlen(command); - if (environment) { env = _php_array_to_envp(environment, is_persistent); } else { @@ -744,7 +779,7 @@ PHP_FUNCTION(proc_open) } } - cmdw = php_win32_cp_conv_any_to_w(command, command_len, &tmp_len); + cmdw = php_win32_cp_conv_any_to_w(command, strlen(command), &tmp_len); if (!cmdw) { php_error_docref(NULL, E_WARNING, "Command conversion failed"); goto exit_fail; @@ -852,10 +887,16 @@ PHP_FUNCTION(proc_open) php_ignore_value(chdir(cwd)); } - if (env.envarray) { - execle("/bin/sh", "sh", "-c", command, NULL, env.envarray); + if (argv) { + /* execvpe() is non-portable, use environ instead. */ + environ = env.envarray; + execvp(command, argv); } else { - execl("/bin/sh", "sh", "-c", command, NULL); + if (env.envarray) { + execle("/bin/sh", "sh", "-c", command, NULL, env.envarray); + } else { + execl("/bin/sh", "sh", "-c", command, NULL); + } } _exit(127); @@ -960,6 +1001,17 @@ PHP_FUNCTION(proc_open) } } +#ifndef PHP_WIN32 + if (argv) { + char **arg = argv; + while (*arg != NULL) { + efree(*arg); + arg++; + } + efree(argv); + } +#endif + efree(descriptors); ZVAL_RES(return_value, zend_register_resource(proc, le_proc_open)); return; @@ -972,6 +1024,15 @@ PHP_FUNCTION(proc_open) free(cwdw); free(cmdw); free(envpw); +#else + if (argv) { + char **arg = argv; + while (*arg != NULL) { + efree(*arg); + arg++; + } + efree(argv); + } #endif #if PHP_CAN_DO_PTS if (dev_ptmx >= 0) { diff --git a/ext/standard/tests/general_functions/proc_open_array.phpt b/ext/standard/tests/general_functions/proc_open_array.phpt new file mode 100644 index 0000000000000..d716a3bea5a5f --- /dev/null +++ b/ext/standard/tests/general_functions/proc_open_array.phpt @@ -0,0 +1,30 @@ +--TEST-- +Using proc_open() with a command array (no shell) +--FILE-- + ['pipe', 'r'], + 1 => ['pipe', 'w'], + 2 => ['pipe', 'w'], +]; + +// Command array cannot be empty +$proc = proc_open([], $ds, $pipes); + +$proc = proc_open([$php, '-r', 'echo "Hello World!\n";'], $ds, $pipes); +var_dump(fgets($pipes[1])); +proc_close($proc); + +$env = ['FOOBARENV' => 'foobar']; +$proc = proc_open([$php, '-r', 'echo getenv("FOOBARENV");'], $ds, $pipes, null, $env); +var_dump(fgets($pipes[1])); +proc_close($proc); + +?> +--EXPECTF-- +Warning: proc_open(): Command array must have at least one element in %s on line %d +string(13) "Hello World! +" +string(6) "foobar" From 26601bd50c9c147d6cde172e16cc8cd0a8d2f55c Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 24 Jun 2019 13:24:50 +0200 Subject: [PATCH 2/6] Implement proc_open() with array on Windows This is a blind implementation... --- ext/standard/proc_open.c | 66 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index d5bbd124cd82d..f6cd22f8cae18 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -34,6 +34,7 @@ #include "php_globals.h" #include "SAPI.h" #include "main/php_network.h" +#include "zend_smart_string.h" #if HAVE_SYS_WAIT_H #include @@ -399,6 +400,64 @@ struct php_proc_open_descriptor_item { }; /* }}} */ +#ifdef PHP_WIN32 +static void append_backslashes(smart_string *str, size_t num_bs) { + size_t i; + for (i = 0; i < num_bs; i++) { + smart_string_appendc(str, '\\'); + } +} + +/* See https://docs.microsoft.com/en-us/cpp/cpp/parsing-cpp-command-line-arguments */ +static void append_win_escaped_arg(smart_string *str, char *arg) { + char c; + size_t num_bs = 0; + smart_string_appendc(str, '"'); + while ((c = *arg)) { + if (c == '\\') { + num_bs++; + } else { + if (c == '"') { + /* Backslashes before " need to be doubled. */ + num_bs = num_bs * 2 + 1; + } + append_backslashes(str, num_bs); + smart_string_appendc(str, c); + num_bs = 0; + } + arg++; + } + append_backslashes(str, num_bs * 2); + smart_string_appendc(str, '"'); +} + +static char *create_win_command_from_args(HashTable *args) { + smart_string str = {0}; + zval *arg_zv; + zend_bool is_prog_name = 1; + + ZEND_HASH_FOREACH_VAL(args, arg_zv) { + zend_string *arg_str = zval_try_get_string(arg_zv); + if (!arg_str) { + smart_string_free(&str); + return NULL; + } + + if (!is_prog_name) { + smart_string_appendc(&str, ' '); + } + + // TODO Do we need special handling for the program name? + append_win_escaped_arg(&str, ZSTR_VAL(arg_str)); + + is_prog_name = 0; + zend_string_release(arg_str); + } ZEND_HASH_FOREACH_END(); + smart_string_0(&str); + return str.c; +} +#endif + /* {{{ proto resource proc_open(string command, array descriptorspec, array &pipes [, string cwd [, array env [, array other_options]]]) Run a process with more control over it's file descriptors */ PHP_FUNCTION(proc_open) @@ -463,7 +522,11 @@ PHP_FUNCTION(proc_open) } #ifdef PHP_WIN32 - TODO + bypass_shell = 1; + command = create_win_command_from_args(Z_ARRVAL_P(command_zv)); + if (!command) { + return; + } #else argv = safe_emalloc(sizeof(char *), num_elems + 1, 0); i = 0; @@ -501,6 +564,7 @@ PHP_FUNCTION(proc_open) } } + /* TODO Deprecate in favor of array command? */ item = zend_hash_str_find(Z_ARRVAL_P(other_options), "bypass_shell", sizeof("bypass_shell") - 1); if (item != NULL) { if (Z_TYPE_P(item) == IS_TRUE || ((Z_TYPE_P(item) == IS_LONG) && Z_LVAL_P(item))) { From 0ca151f4a1f8e8f80d8af7033c9adf0bf844bf3d Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 24 Jun 2019 13:54:31 +0200 Subject: [PATCH 3/6] Avoid warning for uninitialized command --- ext/standard/proc_open.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index f6cd22f8cae18..23653cc670ea2 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -463,7 +463,7 @@ static char *create_win_command_from_args(HashTable *args) { PHP_FUNCTION(proc_open) { zval *command_zv; - char *command, *cwd=NULL; + char *command = NULL, *cwd = NULL; size_t cwd_len = 0; zval *descriptorspec; zval *pipes; @@ -549,6 +549,9 @@ PHP_FUNCTION(proc_open) zend_string_release(arg_str); } ZEND_HASH_FOREACH_END(); argv[i] = NULL; + + /* As the array is non-empty, we should have found a command. */ + ZEND_ASSERT(command); #endif } else { convert_to_string(command_zv); From de8166fa58de93e59303f1131a28e8b774a65a4e Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 24 Jun 2019 14:00:20 +0200 Subject: [PATCH 4/6] Fix environment inheritance --- ext/standard/proc_open.c | 4 ++- .../general_functions/proc_open_array.phpt | 35 ++++++++++++++----- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index 23653cc670ea2..59e6a3c90db75 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -956,7 +956,9 @@ PHP_FUNCTION(proc_open) if (argv) { /* execvpe() is non-portable, use environ instead. */ - environ = env.envarray; + if (env.envarray) { + environ = env.envarray; + } execvp(command, argv); } else { if (env.envarray) { diff --git a/ext/standard/tests/general_functions/proc_open_array.phpt b/ext/standard/tests/general_functions/proc_open_array.phpt index d716a3bea5a5f..859ec85b76cad 100644 --- a/ext/standard/tests/general_functions/proc_open_array.phpt +++ b/ext/standard/tests/general_functions/proc_open_array.phpt @@ -10,21 +10,40 @@ $ds = [ 2 => ['pipe', 'w'], ]; -// Command array cannot be empty +echo "Empty command array:"; $proc = proc_open([], $ds, $pipes); +echo "\nBasic usage:\n"; $proc = proc_open([$php, '-r', 'echo "Hello World!\n";'], $ds, $pipes); -var_dump(fgets($pipes[1])); +fpassthru($pipes[1]); proc_close($proc); -$env = ['FOOBARENV' => 'foobar']; -$proc = proc_open([$php, '-r', 'echo getenv("FOOBARENV");'], $ds, $pipes, null, $env); -var_dump(fgets($pipes[1])); +putenv('ENV_1=ENV_1'); +$env = ['ENV_2' => 'ENV_2']; +$cmd = [$php, '-r', 'var_dump(getenv("ENV_1"), getenv("ENV_2"));']; + +echo "\nEnvironment inheritance:\n"; +$proc = proc_open($cmd, $ds, $pipes); +fpassthru($pipes[1]); +proc_close($proc); + +echo "\nExplicit environment:\n"; +$proc = proc_open($cmd, $ds, $pipes, null, $env); +fpassthru($pipes[1]); proc_close($proc); ?> --EXPECTF-- +Empty command array: Warning: proc_open(): Command array must have at least one element in %s on line %d -string(13) "Hello World! -" -string(6) "foobar" + +Basic usage: +Hello World! + +Environment inheritance: +string(5) "ENV_1" +bool(false) + +Explicit environment: +bool(false) +string(5) "ENV_2" From df6ee820870eaee8a7fc41ac5286d818cff9be8c Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Mon, 24 Jun 2019 14:11:32 +0200 Subject: [PATCH 5/6] Add some more test cases (especially for Windows) --- .../general_functions/proc_open_array.phpt | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/ext/standard/tests/general_functions/proc_open_array.phpt b/ext/standard/tests/general_functions/proc_open_array.phpt index 859ec85b76cad..eda66dc3bef2b 100644 --- a/ext/standard/tests/general_functions/proc_open_array.phpt +++ b/ext/standard/tests/general_functions/proc_open_array.phpt @@ -32,6 +32,22 @@ $proc = proc_open($cmd, $ds, $pipes, null, $env); fpassthru($pipes[1]); proc_close($proc); +echo "\nCheck that arguments are correctly passed through:\n"; +$args = [ + "Simple", + "White space\ttab\nnewline", + "Nul\0bytes", // Should be cut off + '"Quoted"', + 'Qu"ot"ed', + '\\Back\\slash\\', + '\\\\Back\\\\slash\\\\', + '\\"Qu\\"ot\\"ed\\"', +]; +$cmd = [$php, '-r', 'var_export(array_slice($argv, 1));', '--', ...$args]; +$proc = proc_open($cmd, $ds, $pipes); +fpassthru($pipes[1]); +proc_close($proc); + ?> --EXPECTF-- Empty command array: @@ -47,3 +63,16 @@ bool(false) Explicit environment: bool(false) string(5) "ENV_2" + +Check that arguments are correctly passed through: +array ( + 0 => 'Simple', + 1 => 'White space tab +newline', + 2 => 'Nul', + 3 => '"Quoted"', + 4 => 'Qu"ot"ed', + 5 => '\\Back\\slash\\', + 6 => '\\\\Back\\\\slash\\\\', + 7 => '\\"Qu\\"ot\\"ed\\"', +) From ad5e89d33bd4c12461b198b635015ea0736a38b2 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 25 Jun 2019 10:21:46 +0200 Subject: [PATCH 6/6] proc_open: Make null bytes in arguments an error --- ext/standard/proc_open.c | 44 +++++++++++++------ .../general_functions/proc_open_array.phpt | 29 ++++++++---- 2 files changed, 51 insertions(+), 22 deletions(-) diff --git a/ext/standard/proc_open.c b/ext/standard/proc_open.c index 59e6a3c90db75..4644b60794e28 100644 --- a/ext/standard/proc_open.c +++ b/ext/standard/proc_open.c @@ -400,6 +400,22 @@ struct php_proc_open_descriptor_item { }; /* }}} */ +static zend_string *get_valid_arg_string(zval *zv, int elem_num) { + zend_string *str = zval_get_string(zv); + if (!str) { + return NULL; + } + + if (strlen(ZSTR_VAL(str)) != ZSTR_LEN(str)) { + php_error_docref(NULL, E_WARNING, + "Command array element %d contains a null byte", elem_num); + zend_string_release(str); + return NULL; + } + + return str; +} + #ifdef PHP_WIN32 static void append_backslashes(smart_string *str, size_t num_bs) { size_t i; @@ -435,9 +451,10 @@ static char *create_win_command_from_args(HashTable *args) { smart_string str = {0}; zval *arg_zv; zend_bool is_prog_name = 1; + int elem_num = 0; ZEND_HASH_FOREACH_VAL(args, arg_zv) { - zend_string *arg_str = zval_try_get_string(arg_zv); + zend_string *arg_str = get_valid_arg_string(arg_zv, ++elem_num); if (!arg_str) { smart_string_free(&str); return NULL; @@ -447,7 +464,6 @@ static char *create_win_command_from_args(HashTable *args) { smart_string_appendc(&str, ' '); } - // TODO Do we need special handling for the program name? append_win_escaped_arg(&str, ZSTR_VAL(arg_str)); is_prog_name = 0; @@ -513,6 +529,8 @@ PHP_FUNCTION(proc_open) Z_PARAM_ARRAY_EX(other_options, 1, 0) ZEND_PARSE_PARAMETERS_END_EX(RETURN_FALSE); + memset(&env, 0, sizeof(env)); + if (Z_TYPE_P(command_zv) == IS_ARRAY) { zval *arg_zv; uint32_t num_elems = zend_hash_num_elements(Z_ARRVAL_P(command_zv)); @@ -525,20 +543,16 @@ PHP_FUNCTION(proc_open) bypass_shell = 1; command = create_win_command_from_args(Z_ARRVAL_P(command_zv)); if (!command) { - return; + RETURN_FALSE; } #else argv = safe_emalloc(sizeof(char *), num_elems + 1, 0); i = 0; ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(command_zv), arg_zv) { - zend_string *arg_str = zval_try_get_string(arg_zv); + zend_string *arg_str = get_valid_arg_string(arg_zv, i + 1); if (!arg_str) { - int j; - for (j = 0; j < i; j++) { - efree(argv[j]); - } - efree(argv); - return; + argv[i] = NULL; + goto exit_fail; } if (i == 0) { @@ -593,8 +607,6 @@ PHP_FUNCTION(proc_open) if (environment) { env = _php_array_to_envp(environment, is_persistent); - } else { - memset(&env, 0, sizeof(env)); } ndescriptors_array = zend_hash_num_elements(Z_ARRVAL_P(descriptorspec)); @@ -1086,9 +1098,13 @@ PHP_FUNCTION(proc_open) return; exit_fail: - efree(descriptors); + if (descriptors) { + efree(descriptors); + } _php_free_envp(env, is_persistent); - pefree(command, is_persistent); + if (command) { + pefree(command, is_persistent); + } #ifdef PHP_WIN32 free(cwdw); free(cmdw); diff --git a/ext/standard/tests/general_functions/proc_open_array.phpt b/ext/standard/tests/general_functions/proc_open_array.phpt index eda66dc3bef2b..a975c02a993cf 100644 --- a/ext/standard/tests/general_functions/proc_open_array.phpt +++ b/ext/standard/tests/general_functions/proc_open_array.phpt @@ -11,7 +11,13 @@ $ds = [ ]; echo "Empty command array:"; -$proc = proc_open([], $ds, $pipes); +var_dump(proc_open([], $ds, $pipes)); + +echo "\nNul byte in program name:"; +var_dump(proc_open(["php\0oops"], $ds, $pipes)); + +echo "\nNul byte in argument:"; +var_dump(proc_open(["php", "arg\0oops"], $ds, $pipes)); echo "\nBasic usage:\n"; $proc = proc_open([$php, '-r', 'echo "Hello World!\n";'], $ds, $pipes); @@ -36,7 +42,6 @@ echo "\nCheck that arguments are correctly passed through:\n"; $args = [ "Simple", "White space\ttab\nnewline", - "Nul\0bytes", // Should be cut off '"Quoted"', 'Qu"ot"ed', '\\Back\\slash\\', @@ -52,6 +57,15 @@ proc_close($proc); --EXPECTF-- Empty command array: Warning: proc_open(): Command array must have at least one element in %s on line %d +bool(false) + +Nul byte in program name: +Warning: proc_open(): Command array element 1 contains a null byte in %s on line %d +bool(false) + +Nul byte in argument: +Warning: proc_open(): Command array element 2 contains a null byte in %s on line %d +bool(false) Basic usage: Hello World! @@ -69,10 +83,9 @@ array ( 0 => 'Simple', 1 => 'White space tab newline', - 2 => 'Nul', - 3 => '"Quoted"', - 4 => 'Qu"ot"ed', - 5 => '\\Back\\slash\\', - 6 => '\\\\Back\\\\slash\\\\', - 7 => '\\"Qu\\"ot\\"ed\\"', + 2 => '"Quoted"', + 3 => 'Qu"ot"ed', + 4 => '\\Back\\slash\\', + 5 => '\\\\Back\\\\slash\\\\', + 6 => '\\"Qu\\"ot\\"ed\\"', )