[php-src] master: Fix GH-14695: Strictly validate invalid upload_max_filesize and post_max_size values
Weilin Du <[email protected]>
| Newsgroups | gmane.comp.php.cvs.general |
|---|---|
| Message-ID | <[email protected]> |
Author: Weilin Du (LamentXU123)
Date: 2026-06-27T23:01:42+08:00
Commit: https://github.com/php/php-src/commit/998bce0f0fe517bc8fbd7a4699111a8ff8f02846
Raw diff: https://github.com/php/php-src/commit/998bce0f0fe517bc8fbd7a4699111a8ff8f02846.diff
Fix GH-14695: Strictly validate invalid upload_max_filesize and post_max_size values
Changed paths:
A Zend/tests/zend_ini/gh14695.phpt
A Zend/tests/zend_ini/gh14695_empty.phpt
M NEWS
M Zend/zend_ini.c
M Zend/zend_ini.h
M ext/standard/http.c
M ext/standard/tests/http/request_parse_body/multipart_options_invalid_quantity.phpt
M main/main.c
Diff:
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--
+<?php
+
+var_dump(ini_get('upload_max_filesize'));
+var_dump(ini_get('post_max_size'));
+
+?>
+--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--
+<?php
+
+var_dump(ini_get('upload_max_filesize'));
+
+?>
+--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--
<?php
-try {
- request_parse_body(options: [
- 'upload_max_filesize' => '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)