diff --git a/UPGRADING b/UPGRADING index d0bb9bae6131b..b7372ecef082d 100644 --- a/UPGRADING +++ b/UPGRADING @@ -42,6 +42,12 @@ PHP 8.5 UPGRADE NOTES . ldap_get_option() and ldap_set_option() now throw a ValueError when passing an invalid option. +- PCNTL: + . pcntl_exec() now throws ValueErrors when entries of the $args parameter + contain null bytes. + . pcntl_exec() now throws ValueErrors when entries or keys of the + $env_vars parameter contain null bytes. + - SPL: . ArrayObject no longer accepts enums, as modifying the $name or $value properties can break engine assumptions. @@ -91,6 +97,9 @@ PHP 8.5 UPGRADE NOTES gzfile, gzopen and readgzfile functions had been changed from int to boolean. +- PCNTL: + . pcntl_exec() now has a formal return type of false. + - PDO_PGSQL: . PDO::pgsqlCopyFromArray also supports inputs as Iterable. . Pdo\Pgsql::setAttribute and Pdo\Pgsql::prepare supports diff --git a/ext/pcntl/pcntl.c b/ext/pcntl/pcntl.c index dc8634be38240..4b5aa561d59ec 100644 --- a/ext/pcntl/pcntl.c +++ b/ext/pcntl/pcntl.c @@ -620,44 +620,44 @@ PHP_FUNCTION(pcntl_wstopsig) /* {{{ Executes specified program in current process space as defined by exec(2) */ PHP_FUNCTION(pcntl_exec) { - zval *args = NULL, *envs = NULL; + zval *args = NULL; + HashTable *env_vars_ht = NULL; zval *element; - HashTable *args_hash, *envs_hash; - int argc = 0, argi = 0; - int envc = 0, envi = 0; char **argv = NULL, **envp = NULL; - char **current_arg, **pair; - size_t pair_length; - zend_string *key; char *path; size_t path_len; - zend_ulong key_num; ZEND_PARSE_PARAMETERS_START(1, 3) Z_PARAM_PATH(path, path_len) Z_PARAM_OPTIONAL Z_PARAM_ARRAY(args) - Z_PARAM_ARRAY(envs) + Z_PARAM_ARRAY_HT(env_vars_ht) ZEND_PARSE_PARAMETERS_END(); - if (ZEND_NUM_ARGS() > 1) { + if (args != NULL) { + // TODO Check array is a list? /* Build argument list */ SEPARATE_ARRAY(args); - args_hash = Z_ARRVAL_P(args); - argc = zend_hash_num_elements(args_hash); + const HashTable *args_ht = Z_ARRVAL_P(args); + uint32_t argc = zend_hash_num_elements(args_ht); + /* We want a NULL terminated array of char* with the first entry being the path, + * followed by the arguments */ argv = safe_emalloc((argc + 2), sizeof(char *), 0); - *argv = path; - current_arg = argv+1; - ZEND_HASH_FOREACH_VAL(args_hash, element) { - if (argi >= argc) break; + argv[0] = path; + char **current_arg = argv+1; + ZEND_HASH_FOREACH_VAL(args_ht, element) { if (!try_convert_to_string(element)) { efree(argv); RETURN_THROWS(); } + if (zend_str_has_nul_byte(Z_STR_P(element))) { + zend_argument_value_error(2, "individual argument must not contain null bytes"); + efree(argv); + RETURN_THROWS(); + } *current_arg = Z_STRVAL_P(element); - argi++; current_arg++; } ZEND_HASH_FOREACH_END(); *current_arg = NULL; @@ -667,39 +667,51 @@ PHP_FUNCTION(pcntl_exec) argv[1] = NULL; } - if ( ZEND_NUM_ARGS() == 3 ) { + if (env_vars_ht != NULL) { /* Build environment pair list */ - SEPARATE_ARRAY(envs); - envs_hash = Z_ARRVAL_P(envs); - envc = zend_hash_num_elements(envs_hash); + char **pair; + zend_ulong key_num; + zend_string *key; - size_t envp_len = (envc + 1); + /* We want a NULL terminated array of char* */ + size_t envp_len = zend_hash_num_elements(env_vars_ht) + 1; pair = envp = safe_emalloc(envp_len, sizeof(char *), 0); memset(envp, 0, sizeof(char *) * envp_len); - ZEND_HASH_FOREACH_KEY_VAL(envs_hash, key_num, key, element) { - if (envi >= envc) break; - if (!key) { - key = zend_long_to_str(key_num); - } else { - zend_string_addref(key); + ZEND_HASH_FOREACH_KEY_VAL(env_vars_ht, key_num, key, element) { + zend_string *element_str = zval_try_get_string(element); + if (element_str == NULL) { + goto cleanup_env_vars; } - if (!try_convert_to_string(element)) { - zend_string_release(key); + if (zend_str_has_nul_byte(element_str)) { + zend_argument_value_error(3, "value for environment variable must not contain null bytes"); + zend_string_release_ex(element_str, false); goto cleanup_env_vars; } + /* putenv() allows integer environment variables */ + if (!key) { + key = zend_long_to_str((zend_long) key_num); + } else { + if (zend_str_has_nul_byte(key)) { + zend_argument_value_error(3, "name for environment variable must not contain null bytes"); + zend_string_release_ex(element_str, false); + goto cleanup_env_vars; + } + zend_string_addref(key); + } + /* Length of element + equal sign + length of key + null */ - ZEND_ASSERT(Z_STRLEN_P(element) < SIZE_MAX && ZSTR_LEN(key) < SIZE_MAX); - *pair = safe_emalloc(Z_STRLEN_P(element) + 1, sizeof(char), ZSTR_LEN(key) + 1); - pair_length = Z_STRLEN_P(element) + ZSTR_LEN(key) + 2; - strlcpy(*pair, ZSTR_VAL(key), ZSTR_LEN(key) + 1); - strlcat(*pair, "=", pair_length); - strlcat(*pair, Z_STRVAL_P(element), pair_length); + *pair = safe_emalloc(ZSTR_LEN(element_str) + 1, sizeof(char), ZSTR_LEN(key) + 1); + /* Copy key=element + final null byte into buffer */ + memcpy(*pair, ZSTR_VAL(key), ZSTR_LEN(key)); + (*pair)[ZSTR_LEN(key)] = '='; + /* Copy null byte */ + memcpy(*pair + ZSTR_LEN(key) + 1, ZSTR_VAL(element_str), ZSTR_LEN(element_str) + 1); /* Cleanup */ - zend_string_release_ex(key, 0); - envi++; + zend_string_release_ex(key, false); + zend_string_release_ex(element_str, false); pair++; } ZEND_HASH_FOREACH_END(); *(pair) = NULL; diff --git a/ext/pcntl/pcntl.stub.php b/ext/pcntl/pcntl.stub.php index bdae57414b4c3..c62a40139d9ed 100644 --- a/ext/pcntl/pcntl.stub.php +++ b/ext/pcntl/pcntl.stub.php @@ -1055,7 +1055,7 @@ function pcntl_wtermsig(int $status): int|false {} function pcntl_wstopsig(int $status): int|false {} - function pcntl_exec(string $path, array $args = [], array $env_vars = []): bool {} + function pcntl_exec(string $path, array $args = [], array $env_vars = []): false {} function pcntl_alarm(int $seconds): int {} diff --git a/ext/pcntl/pcntl_arginfo.h b/ext/pcntl/pcntl_arginfo.h index 9ae35091c4098..8da7cb70a6aa7 100644 --- a/ext/pcntl/pcntl_arginfo.h +++ b/ext/pcntl/pcntl_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 897062ad1dfe06326e561429509360174820379e */ + * Stub hash: c637fe2de641cfd8f0ff83a908ac59bf63a68e44 */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_fork, 0, 0, IS_LONG, 0) ZEND_END_ARG_INFO() @@ -83,7 +83,7 @@ ZEND_END_ARG_INFO() #define arginfo_pcntl_wstopsig arginfo_pcntl_wexitstatus -ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_exec, 0, 1, _IS_BOOL, 0) +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_pcntl_exec, 0, 1, IS_FALSE, 0) ZEND_ARG_TYPE_INFO(0, path, IS_STRING, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, args, IS_ARRAY, 0, "[]") ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, env_vars, IS_ARRAY, 0, "[]") diff --git a/ext/pcntl/tests/pcntl_exec_strings_with_nul_bytes.phpt b/ext/pcntl/tests/pcntl_exec_strings_with_nul_bytes.phpt new file mode 100644 index 0000000000000..5bf4c9188e941 --- /dev/null +++ b/ext/pcntl/tests/pcntl_exec_strings_with_nul_bytes.phpt @@ -0,0 +1,36 @@ +--TEST-- +pcntl_exec(): Test cleanup after value that contain null bytes has been encountered for $args and $env_vars. +--EXTENSIONS-- +pcntl +--FILE-- +getMessage(), "\n"; +} + +try { + pcntl_exec( + 'cmd', + ['-n'], + ['var1' => 'value1', 'var2' => "value\0null\0byte"], + ); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), "\n"; +} + +try { + pcntl_exec( + 'cmd', + ['-n'], + ['var1' => 'value1', "key\0null\0byte" => "value2"], + ); +} catch (Throwable $e) { + echo $e::class, ': ', $e->getMessage(), "\n"; +} +?> +--EXPECT-- +ValueError: pcntl_exec(): Argument #2 ($args) individual argument must not contain null bytes +ValueError: pcntl_exec(): Argument #3 ($env_vars) value for environment variable must not contain null bytes +ValueError: pcntl_exec(): Argument #3 ($env_vars) name for environment variable must not contain null bytes