From 4dc40ed1d4c7d067fe68c5ab5f748d759ddb0612 Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Fri, 3 Jul 2026 18:51:01 +0200 Subject: [PATCH 1/3] ext/session: reject non-numeric session.upload_progress.freq OnUpdateRfc1867Freq used ZEND_ATOL(), which silently truncates trailing garbage (e.g. "5 apples" becomes 5), the same parsing bug recently fixed for session.cookie_lifetime. Switch to is_numeric_string_ex() to validate the numeric portion strictly, still allowing the optional trailing "%" for percentage mode. --- ext/session/session.c | 14 ++++++++++++-- .../tests/rfc1867_invalid_settings_3.phpt | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 ext/session/tests/rfc1867_invalid_settings_3.phpt diff --git a/ext/session/session.c b/ext/session/session.c index dd968d453bda..a8896c804ba5 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -867,14 +867,24 @@ static PHP_INI_MH(OnUpdateSessionDivisor) static PHP_INI_MH(OnUpdateRfc1867Freq) { - int new_freq = ZEND_ATOL(ZSTR_VAL(new_value)); + const char *str = ZSTR_VAL(new_value); + size_t len = ZSTR_LEN(new_value); + bool is_percentage = len > 0 && str[len - 1] == '%'; + size_t numeric_len = is_percentage ? len - 1 : len; + + zend_long new_freq = 0; + uint8_t type = is_numeric_string_ex(str, numeric_len, &new_freq, NULL, false, NULL, NULL); + if (type != IS_LONG) { + php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be of type int"); + return FAILURE; + } if (new_freq < 0) { php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be greater than or equal to 0"); return FAILURE; } - if (ZSTR_LEN(new_value) > 0 && ZSTR_VAL(new_value)[ZSTR_LEN(new_value) - 1] == '%') { + if (is_percentage) { if (new_freq > 100) { php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be less than or equal to 100%%"); return FAILURE; diff --git a/ext/session/tests/rfc1867_invalid_settings_3.phpt b/ext/session/tests/rfc1867_invalid_settings_3.phpt new file mode 100644 index 000000000000..f8f998f43c6e --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_3.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 3: non-numeric freq is rejected instead of silently truncated +--INI-- +session.upload_progress.freq=5 apples +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%" From ea33eeb2de0802f9da8528b9d2fee996130c48d3 Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Fri, 3 Jul 2026 19:03:00 +0200 Subject: [PATCH 2/3] ext/session: add rfc1867_freq test for garbage input in percentage mode Covers the "%"-suffixed form of session.upload_progress.freq, alongside the plain-integer case added in rfc1867_invalid_settings_3.phpt. --- .../tests/rfc1867_invalid_settings_4.phpt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 ext/session/tests/rfc1867_invalid_settings_4.phpt diff --git a/ext/session/tests/rfc1867_invalid_settings_4.phpt b/ext/session/tests/rfc1867_invalid_settings_4.phpt new file mode 100644 index 000000000000..d51a1538ad8a --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_4.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 4: non-numeric freq in percentage mode is rejected instead of silently truncated +--INI-- +session.upload_progress.freq=5 apples% +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%" From 3f78a8979e5170b33f59ff97ba027fe0dadf7227 Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Sat, 4 Jul 2026 22:37:06 +0200 Subject: [PATCH 3/3] ext/session: normalize whitespace handling in session.upload_progress.freq Whitespace surrounding the whole value (leading, or trailing after an optional "%") is tolerated, but whitespace between the number and the "%" suffix (e.g. "5 %") is rejected rather than silently accepted. The stored ini value is also normalized to a whitespace-free canonical form so ini_get() no longer echoes back stray whitespace. --- ext/session/session.c | 24 +++++++++++++++++++ .../rfc1867_freq_leading_whitespace.phpt | 17 +++++++++++++ .../rfc1867_freq_trailing_percent_space.phpt | 17 +++++++++++++ .../tests/rfc1867_invalid_settings_5.phpt | 18 ++++++++++++++ .../tests/rfc1867_invalid_settings_6.phpt | 18 ++++++++++++++ 5 files changed, 94 insertions(+) create mode 100644 ext/session/tests/rfc1867_freq_leading_whitespace.phpt create mode 100644 ext/session/tests/rfc1867_freq_trailing_percent_space.phpt create mode 100644 ext/session/tests/rfc1867_invalid_settings_5.phpt create mode 100644 ext/session/tests/rfc1867_invalid_settings_6.phpt diff --git a/ext/session/session.c b/ext/session/session.c index a8896c804ba5..2a95ab1dc811 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -865,13 +865,31 @@ static PHP_INI_MH(OnUpdateSessionDivisor) return SUCCESS; } +static bool php_session_is_ini_whitespace(char c) +{ + return c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\v' || c == '\f'; +} + static PHP_INI_MH(OnUpdateRfc1867Freq) { const char *str = ZSTR_VAL(new_value); size_t len = ZSTR_LEN(new_value); + + /* Ignore trailing whitespace so it doesn't hide the '%' suffix (leading + * whitespace is already tolerated by is_numeric_string_ex() below). */ + while (len > 0 && php_session_is_ini_whitespace(str[len - 1])) { + len--; + } + bool is_percentage = len > 0 && str[len - 1] == '%'; size_t numeric_len = is_percentage ? len - 1 : len; + /* Whitespace directly before the '%' (e.g. "5 %") is rejected. */ + if (is_percentage && numeric_len > 0 && php_session_is_ini_whitespace(str[numeric_len - 1])) { + php_error_docref(NULL, E_WARNING, "session.upload_progress.freq must be of type int"); + return FAILURE; + } + zend_long new_freq = 0; uint8_t type = is_numeric_string_ex(str, numeric_len, &new_freq, NULL, false, NULL, NULL); if (type != IS_LONG) { @@ -894,6 +912,12 @@ static PHP_INI_MH(OnUpdateRfc1867Freq) PS(rfc1867_freq) = new_freq; } + /* Store a whitespace-free canonical value so ini_get() doesn't echo back stray whitespace. */ + char buf[MAX_LENGTH_OF_LONG + 2]; + int normalized_len = snprintf(buf, sizeof(buf), is_percentage ? ZEND_LONG_FMT "%%" : ZEND_LONG_FMT, new_freq); + entry->value = zend_string_init(buf, (size_t) normalized_len, true); + GC_MAKE_PERSISTENT_LOCAL(entry->value); + return SUCCESS; } diff --git a/ext/session/tests/rfc1867_freq_leading_whitespace.phpt b/ext/session/tests/rfc1867_freq_leading_whitespace.phpt new file mode 100644 index 000000000000..65396e73bbc7 --- /dev/null +++ b/ext/session/tests/rfc1867_freq_leading_whitespace.phpt @@ -0,0 +1,17 @@ +--TEST-- +session rfc1867 freq: leading whitespace is tolerated and stripped from the stored value +--INI-- +session.upload_progress.freq=" 5%" +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECT-- +string(2) "5%" diff --git a/ext/session/tests/rfc1867_freq_trailing_percent_space.phpt b/ext/session/tests/rfc1867_freq_trailing_percent_space.phpt new file mode 100644 index 000000000000..b12d2b87afac --- /dev/null +++ b/ext/session/tests/rfc1867_freq_trailing_percent_space.phpt @@ -0,0 +1,17 @@ +--TEST-- +session rfc1867 freq: trailing whitespace after the "%" suffix is tolerated +--INI-- +session.upload_progress.freq="5% " +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECT-- +string(2) "5%" diff --git a/ext/session/tests/rfc1867_invalid_settings_5.phpt b/ext/session/tests/rfc1867_invalid_settings_5.phpt new file mode 100644 index 000000000000..71e467e37911 --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_5.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 5: whitespace-only freq is rejected +--INI-- +session.upload_progress.freq=" " +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%" diff --git a/ext/session/tests/rfc1867_invalid_settings_6.phpt b/ext/session/tests/rfc1867_invalid_settings_6.phpt new file mode 100644 index 000000000000..2b4e7f53262d --- /dev/null +++ b/ext/session/tests/rfc1867_invalid_settings_6.phpt @@ -0,0 +1,18 @@ +--TEST-- +session rfc1867 invalid settings 6: whitespace between the number and the "%" suffix is rejected +--INI-- +session.upload_progress.freq="5 %" +error_log= +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: PHP Startup: session.upload_progress.freq must be of type int in Unknown on line 0 +string(%d) "1%"