diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index 85357b0d97e1..f3fece4ed00b 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -246,6 +246,12 @@ PHP 8.4 INTERNALS UPGRADE NOTES g. ext/standard - Added the php_base64_encode_ex() API with flag parameters, value can be PHP_BASE64_NO_PADDING to encode without the padding character '='. + - The php_escape_shell_cmd() now takes a zend_string* instead of a char* + Moreover, providing it with a binary safe string is the responsibility of + the caller now. + - The php_escape_shell_arg() now takes a zend_string* instead of a char* + Moreover, providing it with a binary safe string is the responsibility of + the caller now. ======================== 4. OpCode changes diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c index c6111b49e213..2d719c7e29ec 100644 --- a/ext/mbstring/mbstring.c +++ b/ext/mbstring/mbstring.c @@ -4435,7 +4435,6 @@ PHP_FUNCTION(mb_send_mail) zend_string *str_headers = NULL; size_t i; char *to_r = NULL; - char *force_extra_parameters = INI_STR("mail.force_extra_parameters"); bool suppress_content_type = false; bool suppress_content_transfer_encoding = false; @@ -4653,10 +4652,11 @@ PHP_FUNCTION(mb_send_mail) str_headers = smart_str_extract(&str); + zend_string *force_extra_parameters = zend_ini_str_ex("mail.force_extra_parameters", strlen("mail.force_extra_parameters"), false, NULL); if (force_extra_parameters) { extra_cmd = php_escape_shell_cmd(force_extra_parameters); } else if (extra_cmd) { - extra_cmd = php_escape_shell_cmd(ZSTR_VAL(extra_cmd)); + extra_cmd = php_escape_shell_cmd(extra_cmd); } RETVAL_BOOL(php_mail(to_r, ZSTR_VAL(subject), message, ZSTR_VAL(str_headers), extra_cmd ? ZSTR_VAL(extra_cmd) : NULL)); diff --git a/ext/standard/exec.c b/ext/standard/exec.c index 1b1b0ab9e9ce..5b8ba89de970 100644 --- a/ext/standard/exec.c +++ b/ext/standard/exec.c @@ -279,19 +279,23 @@ PHP_FUNCTION(passthru) *NOT* safe for binary strings */ -PHPAPI zend_string *php_escape_shell_cmd(const char *str) +PHPAPI zend_string *php_escape_shell_cmd(const zend_string *unescaped_cmd) { size_t x, y; - size_t l = strlen(str); - uint64_t estimate = (2 * (uint64_t)l) + 1; zend_string *cmd; #ifndef PHP_WIN32 char *p = NULL; #endif + ZEND_ASSERT(ZSTR_LEN(unescaped_cmd) == strlen(ZSTR_VAL(unescaped_cmd)) && "Must be a binary safe string"); + size_t l = ZSTR_LEN(unescaped_cmd); + const char *str = ZSTR_VAL(unescaped_cmd); + + uint64_t estimate = (2 * (uint64_t)l) + 1; + /* max command line length - two single quotes - \0 byte length */ if (l > cmd_max_len - 2 - 1) { - php_error_docref(NULL, E_ERROR, "Command exceeds the allowed length of %zu bytes", cmd_max_len); + zend_value_error("Command exceeds the allowed length of %zu bytes", cmd_max_len); return ZSTR_EMPTY_ALLOC(); } @@ -367,7 +371,7 @@ PHPAPI zend_string *php_escape_shell_cmd(const char *str) ZSTR_VAL(cmd)[y] = '\0'; if (y > cmd_max_len + 1) { - php_error_docref(NULL, E_ERROR, "Escaped command exceeds the allowed length of %zu bytes", cmd_max_len); + zend_value_error("Escaped command exceeds the allowed length of %zu bytes", cmd_max_len); zend_string_release_ex(cmd, 0); return ZSTR_EMPTY_ALLOC(); } @@ -385,16 +389,20 @@ PHPAPI zend_string *php_escape_shell_cmd(const char *str) /* }}} */ /* {{{ php_escape_shell_arg */ -PHPAPI zend_string *php_escape_shell_arg(const char *str) +PHPAPI zend_string *php_escape_shell_arg(const zend_string *unescaped_arg) { size_t x, y = 0; - size_t l = strlen(str); zend_string *cmd; + + ZEND_ASSERT(ZSTR_LEN(unescaped_arg) == strlen(ZSTR_VAL(unescaped_arg)) && "Must be a binary safe string"); + size_t l = ZSTR_LEN(unescaped_arg); + const char *str = ZSTR_VAL(unescaped_arg); + uint64_t estimate = (4 * (uint64_t)l) + 3; /* max command line length - two single quotes - \0 byte length */ if (l > cmd_max_len - 2 - 1) { - php_error_docref(NULL, E_ERROR, "Argument exceeds the allowed length of %zu bytes", cmd_max_len); + zend_value_error("Argument exceeds the allowed length of %zu bytes", cmd_max_len); return ZSTR_EMPTY_ALLOC(); } @@ -453,7 +461,7 @@ PHPAPI zend_string *php_escape_shell_arg(const char *str) ZSTR_VAL(cmd)[y] = '\0'; if (y > cmd_max_len + 1) { - php_error_docref(NULL, E_ERROR, "Escaped argument exceeds the allowed length of %zu bytes", cmd_max_len); + zend_value_error("Escaped argument exceeds the allowed length of %zu bytes", cmd_max_len); zend_string_release_ex(cmd, 0); return ZSTR_EMPTY_ALLOC(); } @@ -471,18 +479,13 @@ PHPAPI zend_string *php_escape_shell_arg(const char *str) /* {{{ Escape shell metacharacters */ PHP_FUNCTION(escapeshellcmd) { - char *command; - size_t command_len; + zend_string *command; ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_STRING(command, command_len) + Z_PARAM_PATH_STR(command) ZEND_PARSE_PARAMETERS_END(); - if (command_len) { - if (command_len != strlen(command)) { - zend_argument_value_error(1, "must not contain any null bytes"); - RETURN_THROWS(); - } + if (ZSTR_LEN(command)) { RETVAL_STR(php_escape_shell_cmd(command)); } else { RETVAL_EMPTY_STRING(); @@ -493,18 +496,12 @@ PHP_FUNCTION(escapeshellcmd) /* {{{ Quote and escape an argument for use in a shell command */ PHP_FUNCTION(escapeshellarg) { - char *argument; - size_t argument_len; + zend_string *argument; ZEND_PARSE_PARAMETERS_START(1, 1) - Z_PARAM_STRING(argument, argument_len) + Z_PARAM_PATH_STR(argument) ZEND_PARSE_PARAMETERS_END(); - if (argument_len != strlen(argument)) { - zend_argument_value_error(1, "must not contain any null bytes"); - RETURN_THROWS(); - } - RETVAL_STR(php_escape_shell_arg(argument)); } /* }}} */ diff --git a/ext/standard/exec.h b/ext/standard/exec.h index db8c74aa8673..1ad755363e3d 100644 --- a/ext/standard/exec.h +++ b/ext/standard/exec.h @@ -20,8 +20,8 @@ PHP_MINIT_FUNCTION(proc_open); PHP_MINIT_FUNCTION(exec); -PHPAPI zend_string *php_escape_shell_cmd(const char *str); -PHPAPI zend_string *php_escape_shell_arg(const char *str); +PHPAPI zend_string *php_escape_shell_cmd(const zend_string *unescaped_cmd); +PHPAPI zend_string *php_escape_shell_arg(const zend_string *unescaped_arg); PHPAPI int php_exec(int type, const char *cmd, zval *array, zval *return_value); #endif /* EXEC_H */ diff --git a/ext/standard/mail.c b/ext/standard/mail.c index 48ab8da078fe..a424825a40dd 100644 --- a/ext/standard/mail.c +++ b/ext/standard/mail.c @@ -247,7 +247,6 @@ PHP_FUNCTION(mail) HashTable *headers_ht = NULL; size_t to_len, message_len; size_t subject_len, i; - char *force_extra_parameters = INI_STR("mail.force_extra_parameters"); char *to_r, *subject_r; ZEND_PARSE_PARAMETERS_START(3, 5) @@ -312,10 +311,11 @@ PHP_FUNCTION(mail) subject_r = subject; } + zend_string *force_extra_parameters = zend_ini_str_ex("mail.force_extra_parameters", strlen("mail.force_extra_parameters"), false, NULL); if (force_extra_parameters) { extra_cmd = php_escape_shell_cmd(force_extra_parameters); } else if (extra_cmd) { - extra_cmd = php_escape_shell_cmd(ZSTR_VAL(extra_cmd)); + extra_cmd = php_escape_shell_cmd(extra_cmd); } if (php_mail(to_r, subject_r, message, headers_str && ZSTR_LEN(headers_str) ? ZSTR_VAL(headers_str) : NULL, extra_cmd ? ZSTR_VAL(extra_cmd) : NULL)) { diff --git a/ext/standard/tests/general_functions/escapeshellarg_bug71270.phpt b/ext/standard/tests/general_functions/escapeshellarg_bug71270.phpt index c57da3691c55..4460f54ba049 100644 --- a/ext/standard/tests/general_functions/escapeshellarg_bug71270.phpt +++ b/ext/standard/tests/general_functions/escapeshellarg_bug71270.phpt @@ -4,9 +4,15 @@ Test escapeshellarg() allowed argument length getMessage(), PHP_EOL; +} ?> ===DONE=== --EXPECTF-- -Fatal error: escapeshellarg(): Argument exceeds the allowed length of %d bytes in %s on line %d +ValueError: Argument exceeds the allowed length of %d bytes +===DONE=== diff --git a/ext/standard/tests/general_functions/escapeshellcmd_bug71270.phpt b/ext/standard/tests/general_functions/escapeshellcmd_bug71270.phpt index 4686193d4130..3116283000ea 100644 --- a/ext/standard/tests/general_functions/escapeshellcmd_bug71270.phpt +++ b/ext/standard/tests/general_functions/escapeshellcmd_bug71270.phpt @@ -4,9 +4,15 @@ Test escapeshellcmd() allowed argument length getMessage(), PHP_EOL; +} ?> ===DONE=== --EXPECTF-- -Fatal error: escapeshellcmd(): Command exceeds the allowed length of %d bytes in %s on line %d +ValueError: Command exceeds the allowed length of %d bytes +===DONE=== diff --git a/main/main.c b/main/main.c index eae997730333..330f87505bf8 100644 --- a/main/main.c +++ b/main/main.c @@ -652,6 +652,11 @@ static PHP_INI_MH(OnUpdateMailLog) /* {{{ PHP_INI_MH */ static PHP_INI_MH(OnChangeMailForceExtra) { + /* Check that INI setting does not have any nul bytes */ + if (new_value && ZSTR_LEN(new_value) != strlen(ZSTR_VAL(new_value))) { + /* TODO Emit warning? */ + return FAILURE; + } /* Don't allow changing it in htaccess */ if (stage == PHP_INI_STAGE_HTACCESS) { return FAILURE;