From 998bce0f0fe517bc8fbd7a4699111a8ff8f02846 Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Fri, 26 Jun 2026 14:42:49 +0800 Subject: [PATCH 1/4] Fix GH-14695: Strictly validate invalid upload_max_filesize and post_max_size values --- NEWS | 2 + Zend/tests/zend_ini/gh14695.phpt | 17 +++ Zend/tests/zend_ini/gh14695_empty.phpt | 13 ++ Zend/zend_ini.c | 114 +++++++++++++++--- Zend/zend_ini.h | 7 ++ ext/standard/http.c | 10 +- .../multipart_options_invalid_quantity.phpt | 18 +-- main/main.c | 4 +- 8 files changed, 153 insertions(+), 32 deletions(-) create mode 100644 Zend/tests/zend_ini/gh14695.phpt create mode 100644 Zend/tests/zend_ini/gh14695_empty.phpt diff --git a/NEWS b/NEWS index 9df461452c1c..5f1d3b951738 100644 --- a/NEWS +++ b/NEWS @@ -32,6 +32,8 @@ PHP NEWS surrounding property access). (timwolla) . Fixed GH-22422 (zend_arena layout mismatch leaked memory in separately built extensions under AddressSanitizer). (iliaal) + . Added validation for invalid upload_max_filesize and post_max_size values + (GH-14695). (Weilin Du) . TSRM: use local-exec TLS in PIE executables. (henderkes) . perf: make all static extensions use TSRMG_STATIC. (henderkes) . Fixed bug GH-22257 (type confusion in Exception::getTraceAsString()). diff --git a/Zend/tests/zend_ini/gh14695.phpt b/Zend/tests/zend_ini/gh14695.phpt new file mode 100644 index 000000000000..a682c16eff36 --- /dev/null +++ b/Zend/tests/zend_ini/gh14695.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-14695: Invalid upload_max_filesize and post_max_size values are rejected +--INI-- +upload_max_filesize=1zz +post_max_size=bogus +--FILE-- + +--EXPECTF-- +Warning: Invalid "upload_max_filesize" setting. Invalid quantity "1zz": unknown multiplier "z" in %s on line %d +Warning: Invalid "post_max_size" setting. Invalid quantity "bogus": no valid leading digits in %s on line %d +string(2) "2M" +string(2) "8M" diff --git a/Zend/tests/zend_ini/gh14695_empty.phpt b/Zend/tests/zend_ini/gh14695_empty.phpt new file mode 100644 index 000000000000..260644122df2 --- /dev/null +++ b/Zend/tests/zend_ini/gh14695_empty.phpt @@ -0,0 +1,13 @@ +--TEST-- +GH-14695: Empty upload_max_filesize value is rejected +--INI-- +upload_max_filesize= +--FILE-- + +--EXPECTF-- +Warning: Invalid "upload_max_filesize" setting. Invalid quantity "": no valid leading digits in %s on line %d +string(2) "2M" diff --git a/Zend/zend_ini.c b/Zend/zend_ini.c index 093683526d31..78a1974a92ea 100644 --- a/Zend/zend_ini.c +++ b/Zend/zend_ini.c @@ -614,7 +614,12 @@ static const char *zend_ini_consume_quantity_prefix(const char *const digits, co return digits_consumed; } -static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zend_ini_parse_quantity_signed_result_t signed_result, zend_string **errstr) /* {{{ */ +static zend_ulong zend_ini_parse_quantity_internal( + const zend_string *value, + zend_ini_parse_quantity_signed_result_t signed_result, + zend_string **errstr, + bool strict +) /* {{{ */ { char *digits_end = NULL; const char *str = ZSTR_VAL(value); @@ -634,6 +639,18 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen while (digits < str_end && zend_is_whitespace(*(str_end-1))) {--str_end;} if (digits == str_end) { + if (strict) { + if (ZSTR_LEN(value) == 0) { + *errstr = zend_strpprintf(0, "Invalid quantity \"\": no valid leading digits"); + } else { + smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); + smart_str_0(&invalid); + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits", + ZSTR_VAL(invalid.s)); + smart_str_free(&invalid); + } + return 0; + } *errstr = NULL; return 0; } @@ -653,8 +670,13 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); smart_str_0(&invalid); - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", - ZSTR_VAL(invalid.s)); + if (strict) { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits", + ZSTR_VAL(invalid.s)); + } else { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", + ZSTR_VAL(invalid.s)); + } smart_str_free(&invalid); return 0; @@ -690,8 +712,12 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen base = 2; break; default: - *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\", interpreting as \"0\" for backwards compatibility", - digits[1]); + if (strict) { + *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\"", digits[1]); + } else { + *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\", interpreting as \"0\" for backwards compatibility", + digits[1]); + } return 0; } digits += 2; @@ -702,8 +728,13 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); smart_str_0(&invalid); - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix, interpreting as \"0\" for backwards compatibility", - ZSTR_VAL(invalid.s)); + if (strict) { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix", + ZSTR_VAL(invalid.s)); + } else { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix, interpreting as \"0\" for backwards compatibility", + ZSTR_VAL(invalid.s)); + } smart_str_free(&invalid); return 0; @@ -744,8 +775,13 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); smart_str_0(&invalid); - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", - ZSTR_VAL(invalid.s)); + if (strict) { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits", + ZSTR_VAL(invalid.s)); + } else { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", + ZSTR_VAL(invalid.s)); + } smart_str_free(&invalid); return 0; @@ -781,8 +817,13 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen smart_str_append_escaped(&chr, str_end-1, 1); smart_str_0(&chr); - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\", interpreting as \"%s\" for backwards compatibility", - ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s)); + if (strict) { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\"", + ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s)); + } else { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\", interpreting as \"%s\" for backwards compatibility", + ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s)); + } smart_str_free(&invalid); smart_str_free(&interpreted); @@ -815,8 +856,13 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen smart_str_append_escaped(&chr, str_end-1, 1); smart_str_0(&chr); - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\", interpreting as \"%s%s\" for backwards compatibility", - ZSTR_VAL(invalid.s), ZSTR_VAL(interpreted.s), ZSTR_VAL(chr.s)); + if (strict) { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": invalid characters before multiplier \"%s\"", + ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s)); + } else { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\", interpreting as \"%s%s\" for backwards compatibility", + ZSTR_VAL(invalid.s), ZSTR_VAL(interpreted.s), ZSTR_VAL(chr.s)); + } smart_str_free(&invalid); smart_str_free(&interpreted); @@ -833,8 +879,13 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen /* Not specifying the resulting value here because the caller may make * additional conversions. Not specifying the allowed range * because the caller may do narrower range checks. */ - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range, using overflow result for backwards compatibility", - ZSTR_VAL(invalid.s)); + if (strict) { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range", + ZSTR_VAL(invalid.s)); + } else { + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range, using overflow result for backwards compatibility", + ZSTR_VAL(invalid.s)); + } smart_str_free(&invalid); smart_str_free(&interpreted); @@ -850,13 +901,25 @@ static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zen ZEND_API zend_long zend_ini_parse_quantity(const zend_string *value, zend_string **errstr) /* {{{ */ { - return (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr); + return (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr, false); } /* }}} */ ZEND_API zend_ulong zend_ini_parse_uquantity(const zend_string *value, zend_string **errstr) /* {{{ */ { - return zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_UNSIGNED, errstr); + return zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_UNSIGNED, errstr, false); +} +/* }}} */ + +ZEND_API zend_result zend_ini_parse_quantity_strict(const zend_string *value, zend_long *result, zend_string **errstr) /* {{{ */ +{ + zend_long retval = (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr, true); + if (*errstr) { + return FAILURE; + } + + *result = retval; + return SUCCESS; } /* }}} */ @@ -974,6 +1037,23 @@ ZEND_API ZEND_INI_MH(OnUpdateLong) /* {{{ */ } /* }}} */ +ZEND_API ZEND_INI_MH(OnUpdateLongStrict) /* {{{ */ +{ + zend_long tmp; + zend_string *errstr; + if (UNEXPECTED(zend_ini_parse_quantity_strict(new_value, &tmp, &errstr) == FAILURE)) { + zend_error(E_WARNING, "Invalid \"%s\" setting. %s", ZSTR_VAL(entry->name), ZSTR_VAL(errstr)); + zend_string_release(errstr); + return FAILURE; + } + + zend_long *p = ZEND_INI_GET_ADDR(); + *p = tmp; + + return SUCCESS; +} +/* }}} */ + ZEND_API ZEND_INI_MH(OnUpdateLongGEZero) /* {{{ */ { zend_long tmp = zend_ini_parse_quantity_warn(new_value, entry->name); diff --git a/Zend/zend_ini.h b/Zend/zend_ini.h index dbe650675b66..a181048ad9ab 100644 --- a/Zend/zend_ini.h +++ b/Zend/zend_ini.h @@ -142,6 +142,12 @@ ZEND_API zend_long zend_ini_parse_quantity(const zend_string *value, zend_string */ ZEND_API zend_ulong zend_ini_parse_uquantity(const zend_string *value, zend_string **errstr); +/** + * Strict variant of zend_ini_parse_quantity. Ill-formatted values fail instead + * of returning a backwards-compatible interpretation. + */ +ZEND_API zend_result zend_ini_parse_quantity_strict(const zend_string *value, zend_long *result, zend_string **errstr); + ZEND_API zend_long zend_ini_parse_quantity_warn(const zend_string *value, zend_string *setting); ZEND_API zend_ulong zend_ini_parse_uquantity_warn(const zend_string *value, zend_string *setting); @@ -207,6 +213,7 @@ END_EXTERN_C() BEGIN_EXTERN_C() ZEND_API ZEND_INI_MH(OnUpdateBool); ZEND_API ZEND_INI_MH(OnUpdateLong); +ZEND_API ZEND_INI_MH(OnUpdateLongStrict); ZEND_API ZEND_INI_MH(OnUpdateLongGEZero); ZEND_API ZEND_INI_MH(OnUpdateReal); /* char* versions */ diff --git a/ext/standard/http.c b/ext/standard/http.c index d65e7a8acaae..667eddaf0be0 100644 --- a/ext/standard/http.c +++ b/ext/standard/http.c @@ -247,17 +247,17 @@ PHP_FUNCTION(http_build_query) } /* }}} */ -static zend_result cache_request_parse_body_option(HashTable *options, zval *option, int cache_offset) +static zend_result cache_request_parse_body_option(zval *option, const char *option_name, int cache_offset) { if (option) { zend_long result; ZVAL_DEREF(option); if (Z_TYPE_P(option) == IS_STRING) { zend_string *errstr; - result = zend_ini_parse_quantity(Z_STR_P(option), &errstr); - if (errstr) { - zend_error(E_WARNING, "%s", ZSTR_VAL(errstr)); + if (UNEXPECTED(zend_ini_parse_quantity_strict(Z_STR_P(option), &result, &errstr) == FAILURE)) { + zend_value_error("Invalid \"%s\" value in $options argument: %s", option_name, ZSTR_VAL(errstr)); zend_string_release(errstr); + return FAILURE; } } else if (Z_TYPE_P(option) == IS_LONG) { result = Z_LVAL_P(option); @@ -290,7 +290,7 @@ static zend_result cache_request_parse_body_options(HashTable *options) #define CHECK_OPTION(name) \ if (zend_string_equals_literal_ci(key, #name)) { \ - if (cache_request_parse_body_option(options, value, REQUEST_PARSE_BODY_OPTION_ ## name) == FAILURE) { \ + if (cache_request_parse_body_option(value, #name, REQUEST_PARSE_BODY_OPTION_ ## name) == FAILURE) { \ return FAILURE; \ } \ continue; \ diff --git a/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt b/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt index b1efa0dbc91a..63514f597b51 100644 --- a/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt +++ b/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt @@ -3,15 +3,17 @@ request_parse_body() invalid quantity --FILE-- '1GB', - ]); -} catch (Throwable $e) { - echo get_class($e), ': ', $e->getMessage(), "\n"; +foreach (['1GB', ''] as $value) { + try { + request_parse_body(options: [ + 'upload_max_filesize' => $value, + ]); + } catch (Throwable $e) { + echo get_class($e), ': ', $e->getMessage(), "\n"; + } } ?> --EXPECTF-- -Warning: Invalid quantity "1GB": unknown multiplier "B", interpreting as "1" for backwards compatibility in %s on line %d -RequestParseBodyException: Request does not provide a content type +ValueError: Invalid "upload_max_filesize" value in $options argument: Invalid quantity "1GB": unknown multiplier "B" +ValueError: Invalid "upload_max_filesize" value in $options argument: Invalid quantity "": no valid leading digits diff --git a/main/main.c b/main/main.c index 6bda55ac8746..15bed5be9880 100644 --- a/main/main.c +++ b/main/main.c @@ -845,8 +845,8 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("open_basedir", NULL, PHP_INI_ALL, OnUpdateBaseDir, open_basedir, php_core_globals, core_globals) STD_PHP_INI_BOOLEAN("file_uploads", "1", PHP_INI_SYSTEM, OnUpdateBool, file_uploads, php_core_globals, core_globals) - STD_PHP_INI_ENTRY("upload_max_filesize", "2M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLong, upload_max_filesize, php_core_globals, core_globals) - STD_PHP_INI_ENTRY("post_max_size", "8M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLong, post_max_size, sapi_globals_struct,sapi_globals) + STD_PHP_INI_ENTRY("upload_max_filesize", "2M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongStrict, upload_max_filesize, php_core_globals, core_globals) + STD_PHP_INI_ENTRY("post_max_size", "8M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongStrict, post_max_size, sapi_globals_struct,sapi_globals) STD_PHP_INI_ENTRY("upload_tmp_dir", NULL, PHP_INI_SYSTEM, OnUpdateStringUnempty, upload_tmp_dir, php_core_globals, core_globals) STD_PHP_INI_ENTRY("max_input_nesting_level", "64", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongGEZero, max_input_nesting_level, php_core_globals, core_globals) STD_PHP_INI_ENTRY("max_input_vars", "1000", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongGEZero, max_input_vars, php_core_globals, core_globals) From a7204686a9a7cb9e1f41a3e307ec092008bf605f Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Sat, 27 Jun 2026 23:33:46 +0800 Subject: [PATCH 2/4] Zend: Revert strict validation of invalid upload_max_filesize and post_max_size values 998bce0 is accidentally pushed directly to master as this should go to a PR branch. This commit revert that. --- NEWS | 2 - Zend/tests/zend_ini/gh14695.phpt | 17 --- Zend/tests/zend_ini/gh14695_empty.phpt | 13 -- Zend/zend_ini.c | 114 +++--------------- Zend/zend_ini.h | 7 -- ext/standard/http.c | 10 +- .../multipart_options_invalid_quantity.phpt | 18 ++- main/main.c | 4 +- 8 files changed, 32 insertions(+), 153 deletions(-) delete mode 100644 Zend/tests/zend_ini/gh14695.phpt delete mode 100644 Zend/tests/zend_ini/gh14695_empty.phpt diff --git a/NEWS b/NEWS index 5f1d3b951738..9df461452c1c 100644 --- a/NEWS +++ b/NEWS @@ -32,8 +32,6 @@ PHP NEWS surrounding property access). (timwolla) . Fixed GH-22422 (zend_arena layout mismatch leaked memory in separately built extensions under AddressSanitizer). (iliaal) - . Added validation for invalid upload_max_filesize and post_max_size values - (GH-14695). (Weilin Du) . TSRM: use local-exec TLS in PIE executables. (henderkes) . perf: make all static extensions use TSRMG_STATIC. (henderkes) . Fixed bug GH-22257 (type confusion in Exception::getTraceAsString()). diff --git a/Zend/tests/zend_ini/gh14695.phpt b/Zend/tests/zend_ini/gh14695.phpt deleted file mode 100644 index a682c16eff36..000000000000 --- a/Zend/tests/zend_ini/gh14695.phpt +++ /dev/null @@ -1,17 +0,0 @@ ---TEST-- -GH-14695: Invalid upload_max_filesize and post_max_size values are rejected ---INI-- -upload_max_filesize=1zz -post_max_size=bogus ---FILE-- - ---EXPECTF-- -Warning: Invalid "upload_max_filesize" setting. Invalid quantity "1zz": unknown multiplier "z" in %s on line %d -Warning: Invalid "post_max_size" setting. Invalid quantity "bogus": no valid leading digits in %s on line %d -string(2) "2M" -string(2) "8M" diff --git a/Zend/tests/zend_ini/gh14695_empty.phpt b/Zend/tests/zend_ini/gh14695_empty.phpt deleted file mode 100644 index 260644122df2..000000000000 --- a/Zend/tests/zend_ini/gh14695_empty.phpt +++ /dev/null @@ -1,13 +0,0 @@ ---TEST-- -GH-14695: Empty upload_max_filesize value is rejected ---INI-- -upload_max_filesize= ---FILE-- - ---EXPECTF-- -Warning: Invalid "upload_max_filesize" setting. Invalid quantity "": no valid leading digits in %s on line %d -string(2) "2M" diff --git a/Zend/zend_ini.c b/Zend/zend_ini.c index 78a1974a92ea..093683526d31 100644 --- a/Zend/zend_ini.c +++ b/Zend/zend_ini.c @@ -614,12 +614,7 @@ static const char *zend_ini_consume_quantity_prefix(const char *const digits, co return digits_consumed; } -static zend_ulong zend_ini_parse_quantity_internal( - const zend_string *value, - zend_ini_parse_quantity_signed_result_t signed_result, - zend_string **errstr, - bool strict -) /* {{{ */ +static zend_ulong zend_ini_parse_quantity_internal(const zend_string *value, zend_ini_parse_quantity_signed_result_t signed_result, zend_string **errstr) /* {{{ */ { char *digits_end = NULL; const char *str = ZSTR_VAL(value); @@ -639,18 +634,6 @@ static zend_ulong zend_ini_parse_quantity_internal( while (digits < str_end && zend_is_whitespace(*(str_end-1))) {--str_end;} if (digits == str_end) { - if (strict) { - if (ZSTR_LEN(value) == 0) { - *errstr = zend_strpprintf(0, "Invalid quantity \"\": no valid leading digits"); - } else { - smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); - smart_str_0(&invalid); - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits", - ZSTR_VAL(invalid.s)); - smart_str_free(&invalid); - } - return 0; - } *errstr = NULL; return 0; } @@ -670,13 +653,8 @@ static zend_ulong zend_ini_parse_quantity_internal( smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); smart_str_0(&invalid); - if (strict) { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits", - ZSTR_VAL(invalid.s)); - } else { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", - ZSTR_VAL(invalid.s)); - } + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", + ZSTR_VAL(invalid.s)); smart_str_free(&invalid); return 0; @@ -712,12 +690,8 @@ static zend_ulong zend_ini_parse_quantity_internal( base = 2; break; default: - if (strict) { - *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\"", digits[1]); - } else { - *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\", interpreting as \"0\" for backwards compatibility", - digits[1]); - } + *errstr = zend_strpprintf(0, "Invalid prefix \"0%c\", interpreting as \"0\" for backwards compatibility", + digits[1]); return 0; } digits += 2; @@ -728,13 +702,8 @@ static zend_ulong zend_ini_parse_quantity_internal( smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); smart_str_0(&invalid); - if (strict) { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix", - ZSTR_VAL(invalid.s)); - } else { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix, interpreting as \"0\" for backwards compatibility", - ZSTR_VAL(invalid.s)); - } + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no digits after base prefix, interpreting as \"0\" for backwards compatibility", + ZSTR_VAL(invalid.s)); smart_str_free(&invalid); return 0; @@ -775,13 +744,8 @@ static zend_ulong zend_ini_parse_quantity_internal( smart_str_append_escaped(&invalid, ZSTR_VAL(value), ZSTR_LEN(value)); smart_str_0(&invalid); - if (strict) { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits", - ZSTR_VAL(invalid.s)); - } else { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", - ZSTR_VAL(invalid.s)); - } + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": no valid leading digits, interpreting as \"0\" for backwards compatibility", + ZSTR_VAL(invalid.s)); smart_str_free(&invalid); return 0; @@ -817,13 +781,8 @@ static zend_ulong zend_ini_parse_quantity_internal( smart_str_append_escaped(&chr, str_end-1, 1); smart_str_0(&chr); - if (strict) { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\"", - ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s)); - } else { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\", interpreting as \"%s\" for backwards compatibility", - ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s)); - } + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": unknown multiplier \"%s\", interpreting as \"%s\" for backwards compatibility", + ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s), ZSTR_VAL(interpreted.s)); smart_str_free(&invalid); smart_str_free(&interpreted); @@ -856,13 +815,8 @@ static zend_ulong zend_ini_parse_quantity_internal( smart_str_append_escaped(&chr, str_end-1, 1); smart_str_0(&chr); - if (strict) { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": invalid characters before multiplier \"%s\"", - ZSTR_VAL(invalid.s), ZSTR_VAL(chr.s)); - } else { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\", interpreting as \"%s%s\" for backwards compatibility", - ZSTR_VAL(invalid.s), ZSTR_VAL(interpreted.s), ZSTR_VAL(chr.s)); - } + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\", interpreting as \"%s%s\" for backwards compatibility", + ZSTR_VAL(invalid.s), ZSTR_VAL(interpreted.s), ZSTR_VAL(chr.s)); smart_str_free(&invalid); smart_str_free(&interpreted); @@ -879,13 +833,8 @@ static zend_ulong zend_ini_parse_quantity_internal( /* Not specifying the resulting value here because the caller may make * additional conversions. Not specifying the allowed range * because the caller may do narrower range checks. */ - if (strict) { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range", - ZSTR_VAL(invalid.s)); - } else { - *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range, using overflow result for backwards compatibility", - ZSTR_VAL(invalid.s)); - } + *errstr = zend_strpprintf(0, "Invalid quantity \"%s\": value is out of range, using overflow result for backwards compatibility", + ZSTR_VAL(invalid.s)); smart_str_free(&invalid); smart_str_free(&interpreted); @@ -901,25 +850,13 @@ static zend_ulong zend_ini_parse_quantity_internal( ZEND_API zend_long zend_ini_parse_quantity(const zend_string *value, zend_string **errstr) /* {{{ */ { - return (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr, false); + return (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr); } /* }}} */ ZEND_API zend_ulong zend_ini_parse_uquantity(const zend_string *value, zend_string **errstr) /* {{{ */ { - return zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_UNSIGNED, errstr, false); -} -/* }}} */ - -ZEND_API zend_result zend_ini_parse_quantity_strict(const zend_string *value, zend_long *result, zend_string **errstr) /* {{{ */ -{ - zend_long retval = (zend_long) zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_SIGNED, errstr, true); - if (*errstr) { - return FAILURE; - } - - *result = retval; - return SUCCESS; + return zend_ini_parse_quantity_internal(value, ZEND_INI_PARSE_QUANTITY_UNSIGNED, errstr); } /* }}} */ @@ -1037,23 +974,6 @@ ZEND_API ZEND_INI_MH(OnUpdateLong) /* {{{ */ } /* }}} */ -ZEND_API ZEND_INI_MH(OnUpdateLongStrict) /* {{{ */ -{ - zend_long tmp; - zend_string *errstr; - if (UNEXPECTED(zend_ini_parse_quantity_strict(new_value, &tmp, &errstr) == FAILURE)) { - zend_error(E_WARNING, "Invalid \"%s\" setting. %s", ZSTR_VAL(entry->name), ZSTR_VAL(errstr)); - zend_string_release(errstr); - return FAILURE; - } - - zend_long *p = ZEND_INI_GET_ADDR(); - *p = tmp; - - return SUCCESS; -} -/* }}} */ - ZEND_API ZEND_INI_MH(OnUpdateLongGEZero) /* {{{ */ { zend_long tmp = zend_ini_parse_quantity_warn(new_value, entry->name); diff --git a/Zend/zend_ini.h b/Zend/zend_ini.h index a181048ad9ab..dbe650675b66 100644 --- a/Zend/zend_ini.h +++ b/Zend/zend_ini.h @@ -142,12 +142,6 @@ ZEND_API zend_long zend_ini_parse_quantity(const zend_string *value, zend_string */ ZEND_API zend_ulong zend_ini_parse_uquantity(const zend_string *value, zend_string **errstr); -/** - * Strict variant of zend_ini_parse_quantity. Ill-formatted values fail instead - * of returning a backwards-compatible interpretation. - */ -ZEND_API zend_result zend_ini_parse_quantity_strict(const zend_string *value, zend_long *result, zend_string **errstr); - ZEND_API zend_long zend_ini_parse_quantity_warn(const zend_string *value, zend_string *setting); ZEND_API zend_ulong zend_ini_parse_uquantity_warn(const zend_string *value, zend_string *setting); @@ -213,7 +207,6 @@ END_EXTERN_C() BEGIN_EXTERN_C() ZEND_API ZEND_INI_MH(OnUpdateBool); ZEND_API ZEND_INI_MH(OnUpdateLong); -ZEND_API ZEND_INI_MH(OnUpdateLongStrict); ZEND_API ZEND_INI_MH(OnUpdateLongGEZero); ZEND_API ZEND_INI_MH(OnUpdateReal); /* char* versions */ diff --git a/ext/standard/http.c b/ext/standard/http.c index 667eddaf0be0..d65e7a8acaae 100644 --- a/ext/standard/http.c +++ b/ext/standard/http.c @@ -247,17 +247,17 @@ PHP_FUNCTION(http_build_query) } /* }}} */ -static zend_result cache_request_parse_body_option(zval *option, const char *option_name, int cache_offset) +static zend_result cache_request_parse_body_option(HashTable *options, zval *option, int cache_offset) { if (option) { zend_long result; ZVAL_DEREF(option); if (Z_TYPE_P(option) == IS_STRING) { zend_string *errstr; - if (UNEXPECTED(zend_ini_parse_quantity_strict(Z_STR_P(option), &result, &errstr) == FAILURE)) { - zend_value_error("Invalid \"%s\" value in $options argument: %s", option_name, ZSTR_VAL(errstr)); + result = zend_ini_parse_quantity(Z_STR_P(option), &errstr); + if (errstr) { + zend_error(E_WARNING, "%s", ZSTR_VAL(errstr)); zend_string_release(errstr); - return FAILURE; } } else if (Z_TYPE_P(option) == IS_LONG) { result = Z_LVAL_P(option); @@ -290,7 +290,7 @@ static zend_result cache_request_parse_body_options(HashTable *options) #define CHECK_OPTION(name) \ if (zend_string_equals_literal_ci(key, #name)) { \ - if (cache_request_parse_body_option(value, #name, REQUEST_PARSE_BODY_OPTION_ ## name) == FAILURE) { \ + if (cache_request_parse_body_option(options, value, REQUEST_PARSE_BODY_OPTION_ ## name) == FAILURE) { \ return FAILURE; \ } \ continue; \ diff --git a/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt b/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt index 63514f597b51..b1efa0dbc91a 100644 --- a/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt +++ b/ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt @@ -3,17 +3,15 @@ request_parse_body() invalid quantity --FILE-- $value, - ]); - } catch (Throwable $e) { - echo get_class($e), ': ', $e->getMessage(), "\n"; - } +try { + request_parse_body(options: [ + 'upload_max_filesize' => '1GB', + ]); +} catch (Throwable $e) { + echo get_class($e), ': ', $e->getMessage(), "\n"; } ?> --EXPECTF-- -ValueError: Invalid "upload_max_filesize" value in $options argument: Invalid quantity "1GB": unknown multiplier "B" -ValueError: Invalid "upload_max_filesize" value in $options argument: Invalid quantity "": no valid leading digits +Warning: Invalid quantity "1GB": unknown multiplier "B", interpreting as "1" for backwards compatibility in %s on line %d +RequestParseBodyException: Request does not provide a content type diff --git a/main/main.c b/main/main.c index 15bed5be9880..6bda55ac8746 100644 --- a/main/main.c +++ b/main/main.c @@ -845,8 +845,8 @@ PHP_INI_BEGIN() STD_PHP_INI_ENTRY("open_basedir", NULL, PHP_INI_ALL, OnUpdateBaseDir, open_basedir, php_core_globals, core_globals) STD_PHP_INI_BOOLEAN("file_uploads", "1", PHP_INI_SYSTEM, OnUpdateBool, file_uploads, php_core_globals, core_globals) - STD_PHP_INI_ENTRY("upload_max_filesize", "2M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongStrict, upload_max_filesize, php_core_globals, core_globals) - STD_PHP_INI_ENTRY("post_max_size", "8M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongStrict, post_max_size, sapi_globals_struct,sapi_globals) + STD_PHP_INI_ENTRY("upload_max_filesize", "2M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLong, upload_max_filesize, php_core_globals, core_globals) + STD_PHP_INI_ENTRY("post_max_size", "8M", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLong, post_max_size, sapi_globals_struct,sapi_globals) STD_PHP_INI_ENTRY("upload_tmp_dir", NULL, PHP_INI_SYSTEM, OnUpdateStringUnempty, upload_tmp_dir, php_core_globals, core_globals) STD_PHP_INI_ENTRY("max_input_nesting_level", "64", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongGEZero, max_input_nesting_level, php_core_globals, core_globals) STD_PHP_INI_ENTRY("max_input_vars", "1000", PHP_INI_SYSTEM|PHP_INI_PERDIR, OnUpdateLongGEZero, max_input_vars, php_core_globals, core_globals) From 5ff2a95341a692a86646292317d61c63c96d5ca4 Mon Sep 17 00:00:00 2001 From: Peter Kokot Date: Sat, 27 Jun 2026 18:36:21 +0200 Subject: [PATCH 3/4] Remove redundant include (#22477) The HAVE_LIBINTL macro is defined only when the ext/gettext is enabled during the build. Also, LC_MESSAGES have been moved to other places since this header was initially included. --- ext/standard/string.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ext/standard/string.c b/ext/standard/string.c index 006adc4467d3..006ec3509a67 100644 --- a/ext/standard/string.c +++ b/ext/standard/string.c @@ -23,10 +23,6 @@ # include #endif -#ifdef HAVE_LIBINTL -# include /* For LC_MESSAGES */ -#endif - #include "scanf.h" #include "zend_API.h" #include "zend_execute.h" @@ -5763,7 +5759,7 @@ PHP_FUNCTION(substr_count) static void php_str_pad_fill(zend_string *result, size_t pad_chars, const char *pad_str, size_t pad_str_len) { char *p = ZSTR_VAL(result) + ZSTR_LEN(result); - + if (pad_str_len == 1) { memset(p, pad_str[0], pad_chars); ZSTR_LEN(result) += pad_chars; @@ -5778,7 +5774,7 @@ static void php_str_pad_fill(zend_string *result, size_t pad_chars, const char * if (p < end) { memcpy(p, pad_str, end - p); } - + ZSTR_LEN(result) += pad_chars; } From efbf47300b1c1f561b2c76b92ddf27bb987a6fbc Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 26 Jun 2026 15:15:47 -0400 Subject: [PATCH 4/4] Fix use-after-free in RecursiveIteratorIterator on reentrant teardown spl_recursive_it_move_forward_ex() tears down the exhausted level after running its sub-iterator, but endChildren() and a sub-iterator's valid() can re-enter through $this->next() and tear that level down first. The no-more-elements branch then dtored a stale iterator pointer, and valid() kept running on a sub-iterator the reentrant call had already freed. Guard the teardown on the level's iterator being unchanged, and hold a reference on the sub-iterator across valid(). Closes GH-22478 --- ext/spl/spl_iterators.c | 18 ++++++- ...ratoriterator_next_during_endchildren.phpt | 42 +++++++++++++++++ ...iveiteratoriterator_next_during_valid.phpt | 47 +++++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt create mode 100644 ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index eb068a50c91f..425bd99f37c0 100644 --- a/ext/spl/spl_iterators.c +++ b/ext/spl/spl_iterators.c @@ -257,6 +257,10 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv zend_class_entry *ce; zval retval, child; zend_object_iterator *sub_iter; + zend_object *sub_object; + uint32_t prev_level; + zend_result valid_result; + bool reentered; SPL_FETCH_SUB_ITERATOR(iterator, object); @@ -276,7 +280,17 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv iterator = object->iterators[object->level].iterator; ZEND_FALLTHROUGH; case RS_START: - if (iterator->funcs->valid(iterator) == FAILURE) { + sub_object = Z_OBJ(object->iterators[object->level].zobject); + prev_level = object->level; + GC_ADDREF(sub_object); + valid_result = iterator->funcs->valid(iterator); + reentered = object->level != prev_level + || object->iterators[object->level].iterator != iterator; + OBJ_RELEASE(sub_object); + if (reentered) { + return; + } + if (valid_result == FAILURE) { break; } object->iterators[object->level].state = RS_TEST; @@ -424,7 +438,7 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv } } } - if (object->level > 0) { + if (object->level > 0 && object->iterators[object->level].iterator == iterator) { zval garbage; ZVAL_COPY_VALUE(&garbage, &object->iterators[object->level].zobject); ZVAL_UNDEF(&object->iterators[object->level].zobject); diff --git a/ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt b/ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt new file mode 100644 index 000000000000..b3c80b2232a8 --- /dev/null +++ b/ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt @@ -0,0 +1,42 @@ +--TEST-- +RecursiveIteratorIterator: next() re-entered from endChildren() must not use-after-free +--FILE-- +data = $d; } + function current(): mixed { return $this->data[$this->pos] ?? null; } + function key(): mixed { return $this->pos; } + function next(): void { $this->pos++; } + function rewind(): void { $this->pos = 0; } + function valid(): bool { return $this->pos < count($this->data); } + function hasChildren(): bool { return is_array($this->current()); } + function getChildren(): RecursiveIterator { return new RIt($this->current()); } +} + +class ReenterRII extends RecursiveIteratorIterator { + public static int $fired = 0; + function endChildren(): void { + if (self::$fired++ === 0) { + $this->next(); + } + } +} + +$tree = [[[1, 2], 3], 4]; +$it = new ReenterRII(new RIt($tree), RecursiveIteratorIterator::SELF_FIRST); +$seen = []; +foreach ($it as $v) { + $seen[] = is_array($v) ? 'array' : $v; + if (count($seen) > 20) { + $seen[] = '...'; + break; + } +} +echo implode(' ', $seen), "\n"; +echo "done\n"; +?> +--EXPECT-- +array array 1 2 4 +done diff --git a/ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt b/ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt new file mode 100644 index 000000000000..a2dcf4695e36 --- /dev/null +++ b/ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt @@ -0,0 +1,47 @@ +--TEST-- +RecursiveIteratorIterator: next() re-entered from a sub-iterator valid() must not use-after-free +--FILE-- +data = $d; $this->depth = $depth; } + function current(): mixed { return $this->data[$this->pos] ?? null; } + function key(): mixed { return $this->pos; } + function next(): void { $this->pos++; } + function rewind(): void { $this->pos = 0; } + function valid(): bool { + if ($this->rii && $this->depth === 2 && $this->pos === count($this->data) && !self::$fired) { + self::$fired = true; + $this->rii->next(); + } + return $this->pos < count($this->data); + } + function hasChildren(): bool { return is_array($this->current()); } + function getChildren(): RecursiveIterator { + $c = new RIt($this->current(), $this->depth + 1); + $c->rii = $this->rii; + return $c; + } +} + +$root = new RIt([[[1, 2], 3], 4]); +$rii = new RecursiveIteratorIterator($root, RecursiveIteratorIterator::SELF_FIRST); +$root->rii = $rii; +$seen = []; +foreach ($rii as $v) { + $seen[] = is_array($v) ? 'array' : $v; + if (count($seen) > 20) { + $seen[] = '...'; + break; + } +} +echo implode(' ', $seen), "\n"; +echo "done\n"; +?> +--EXPECT-- +array array 1 2 3 4 +done