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 = 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 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 '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\\"', )