From 29eb82220cec992ceb881738591ecacf18fd483d Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Fri, 10 Apr 2026 09:32:43 +0200 Subject: [PATCH 1/3] ext/session: fix cookie_lifetime overflow causing INI value desync When session.cookie_lifetime was set to a value larger than maxcookie, OnUpdateCookieLifetime returned SUCCESS without updating the internal long value, causing ini_get() string and PS(cookie_lifetime) to go out of sync. Now the value is properly clamped to maxcookie with both the string and internal long updated consistently, and a warning is emitted. --- ext/session/session.c | 4 +++ ext/session/tests/gh16290.phpt | 5 ++- .../session_cookie_lifetime_overflow.phpt | 34 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) create mode 100644 ext/session/tests/session_cookie_lifetime_overflow.phpt diff --git a/ext/session/session.c b/ext/session/session.c index 5094e7412a88..35a8f472d730 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -711,6 +711,10 @@ static PHP_INI_MH(OnUpdateCookieLifetime) php_error_docref(NULL, E_WARNING, "CookieLifetime cannot be negative"); return FAILURE; } else if (v > maxcookie) { + php_error_docref(NULL, E_WARNING, "CookieLifetime value too large, value was set to the maximum of " ZEND_LONG_FMT, maxcookie); + zend_long *p = ZEND_INI_GET_ADDR(); + *p = maxcookie; + entry->value = zend_long_to_str(maxcookie); return SUCCESS; } diff --git a/ext/session/tests/gh16290.phpt b/ext/session/tests/gh16290.phpt index d341eb47471b..79b72d11fb10 100644 --- a/ext/session/tests/gh16290.phpt +++ b/ext/session/tests/gh16290.phpt @@ -6,8 +6,11 @@ session --FILE-- ---EXPECT-- +--EXPECTF-- +Warning: session_set_cookie_params(): CookieLifetime value too large, value was set to the maximum of %d in %s on line %d DONE diff --git a/ext/session/tests/session_cookie_lifetime_overflow.phpt b/ext/session/tests/session_cookie_lifetime_overflow.phpt new file mode 100644 index 000000000000..d2130c7e7afb --- /dev/null +++ b/ext/session/tests/session_cookie_lifetime_overflow.phpt @@ -0,0 +1,34 @@ +--TEST-- +session.cookie_lifetime overflow value is clamped +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + 0); // positive + +// Valid values still work after clamping +ini_set("session.cookie_lifetime", 200); +var_dump(ini_get("session.cookie_lifetime")); + +ob_end_flush(); +?> +--EXPECTF-- +string(3) "100" + +Warning: ini_set(): CookieLifetime value too large, value was set to the maximum of %d in %s on line %d +bool(true) +bool(true) +string(3) "200" From 2860ac490fa4947347ee01f9ddeaae595eac452c Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Fri, 10 Apr 2026 23:17:22 +0200 Subject: [PATCH 2/3] validation for cookie lifetime --- ext/session/session.c | 16 +++++-- .../session_cookie_lifetime_invalid.phpt | 43 +++++++++++++++++++ .../session_get_cookie_params_basic.phpt | 6 +-- .../session_set_cookie_params_basic.phpt | 2 +- .../session_set_cookie_params_variation1.phpt | 4 +- 5 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 ext/session/tests/session_cookie_lifetime_invalid.phpt diff --git a/ext/session/session.c b/ext/session/session.c index 35a8f472d730..a07ca6651605 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -706,11 +706,21 @@ static PHP_INI_MH(OnUpdateCookieLifetime) #else const zend_long maxcookie = ZEND_LONG_MAX / 2 - 1; #endif - zend_long v = (zend_long)atol(ZSTR_VAL(new_value)); - if (v < 0) { + zend_long lval = 0; + int oflow = 0; + uint8_t type = is_numeric_string_ex(ZSTR_VAL(new_value), ZSTR_LEN(new_value), &lval, NULL, false, &oflow, NULL); + if (type == 0) { + php_error_docref(NULL, E_WARNING, "Invalid value for CookieLifetime"); + return FAILURE; + } else if (type == IS_DOUBLE && oflow == 0) { + php_error_docref(NULL, E_WARNING, "CookieLifetime must be an integer"); + return FAILURE; + } + zend_long v = lval; + if (oflow < 0 || v < 0) { php_error_docref(NULL, E_WARNING, "CookieLifetime cannot be negative"); return FAILURE; - } else if (v > maxcookie) { + } else if (oflow > 0 || v > maxcookie) { php_error_docref(NULL, E_WARNING, "CookieLifetime value too large, value was set to the maximum of " ZEND_LONG_FMT, maxcookie); zend_long *p = ZEND_INI_GET_ADDR(); *p = maxcookie; diff --git a/ext/session/tests/session_cookie_lifetime_invalid.phpt b/ext/session/tests/session_cookie_lifetime_invalid.phpt new file mode 100644 index 000000000000..6c2633088ce8 --- /dev/null +++ b/ext/session/tests/session_cookie_lifetime_invalid.phpt @@ -0,0 +1,43 @@ +--TEST-- +session.cookie_lifetime rejects non-integer values +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: ini_set(): CookieLifetime must be an integer in %s on line %d +string(3) "100" + +Warning: ini_set(): Invalid value for CookieLifetime in %s on line %d +string(3) "100" + +Warning: ini_set(): CookieLifetime cannot be negative in %s on line %d +string(3) "100" + +Warning: ini_set(): CookieLifetime cannot be negative in %s on line %d +string(3) "100" diff --git a/ext/session/tests/session_get_cookie_params_basic.phpt b/ext/session/tests/session_get_cookie_params_basic.phpt index 1c7cdf189fc9..73ffd4c94d1e 100644 --- a/ext/session/tests/session_get_cookie_params_basic.phpt +++ b/ext/session/tests/session_get_cookie_params_basic.phpt @@ -22,7 +22,7 @@ echo "*** Testing session_get_cookie_params() : basic functionality ***\n"; var_dump(session_get_cookie_params()); var_dump(session_set_cookie_params(3600, "/path", "blah", FALSE, FALSE)); var_dump(session_get_cookie_params()); -var_dump(session_set_cookie_params(1234567890, "/guff", "foo", TRUE, TRUE)); +var_dump(session_set_cookie_params(1000000000, "/guff", "foo", TRUE, TRUE)); var_dump(session_get_cookie_params()); var_dump(session_set_cookie_params([ "lifetime" => 123, @@ -40,7 +40,7 @@ var_dump(session_get_cookie_params()); echo "Done"; ob_end_flush(); ?> ---EXPECTF-- +--EXPECT-- *** Testing session_get_cookie_params() : basic functionality *** array(7) { ["lifetime"]=> @@ -78,7 +78,7 @@ array(7) { bool(true) array(7) { ["lifetime"]=> - int(%d) + int(1000000000) ["path"]=> string(5) "/guff" ["domain"]=> diff --git a/ext/session/tests/session_set_cookie_params_basic.phpt b/ext/session/tests/session_set_cookie_params_basic.phpt index 386280d17861..27cfd59b183e 100644 --- a/ext/session/tests/session_set_cookie_params_basic.phpt +++ b/ext/session/tests/session_set_cookie_params_basic.phpt @@ -15,7 +15,7 @@ var_dump(session_set_cookie_params(3600)); var_dump(session_start()); var_dump(session_set_cookie_params(1800)); var_dump(session_destroy()); -var_dump(session_set_cookie_params(1234567890)); +var_dump(session_set_cookie_params(1000000000)); echo "Done"; ob_end_flush(); diff --git a/ext/session/tests/session_set_cookie_params_variation1.phpt b/ext/session/tests/session_set_cookie_params_variation1.phpt index ed0b8dc9755d..ce4b98457bea 100644 --- a/ext/session/tests/session_set_cookie_params_variation1.phpt +++ b/ext/session/tests/session_set_cookie_params_variation1.phpt @@ -24,7 +24,7 @@ var_dump(ini_get("session.cookie_lifetime")); var_dump(session_destroy()); var_dump(ini_get("session.cookie_lifetime")); -var_dump(session_set_cookie_params(1234567890)); +var_dump(session_set_cookie_params(1000000000)); var_dump(ini_get("session.cookie_lifetime")); echo "Done"; @@ -44,5 +44,5 @@ string(4) "3600" bool(true) string(4) "3600" bool(true) -string(10) "1234567890" +string(10) "1000000000" Done From 070407c824a6c8cdf76443d027b91b4867ad993c Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Sat, 11 Apr 2026 11:46:24 +0200 Subject: [PATCH 3/3] removed unnecessary comments --- ext/session/tests/session_cookie_lifetime_invalid.phpt | 8 ++++---- ext/session/tests/session_cookie_lifetime_overflow.phpt | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ext/session/tests/session_cookie_lifetime_invalid.phpt b/ext/session/tests/session_cookie_lifetime_invalid.phpt index 6c2633088ce8..b383fcc492ca 100644 --- a/ext/session/tests/session_cookie_lifetime_invalid.phpt +++ b/ext/session/tests/session_cookie_lifetime_invalid.phpt @@ -13,19 +13,19 @@ ini_set("session.cookie_lifetime", 100); // Float strings are rejected ini_set("session.cookie_lifetime", "1.5"); -var_dump(ini_get("session.cookie_lifetime")); // unchanged +var_dump(ini_get("session.cookie_lifetime")); // Non-numeric strings are rejected ini_set("session.cookie_lifetime", "abc"); -var_dump(ini_get("session.cookie_lifetime")); // unchanged +var_dump(ini_get("session.cookie_lifetime")); // Negative values are rejected ini_set("session.cookie_lifetime", -1); -var_dump(ini_get("session.cookie_lifetime")); // unchanged +var_dump(ini_get("session.cookie_lifetime")); // Negative overflow strings are rejected ini_set("session.cookie_lifetime", "-99999999999999999999"); -var_dump(ini_get("session.cookie_lifetime")); // unchanged +var_dump(ini_get("session.cookie_lifetime")); ob_end_flush(); ?> diff --git a/ext/session/tests/session_cookie_lifetime_overflow.phpt b/ext/session/tests/session_cookie_lifetime_overflow.phpt index d2130c7e7afb..afb0d3bd2eff 100644 --- a/ext/session/tests/session_cookie_lifetime_overflow.phpt +++ b/ext/session/tests/session_cookie_lifetime_overflow.phpt @@ -17,7 +17,7 @@ var_dump(ini_get("session.cookie_lifetime")); ini_set("session.cookie_lifetime", PHP_INT_MAX); $val = (int) ini_get("session.cookie_lifetime"); var_dump($val < PHP_INT_MAX); // clamped, not PHP_INT_MAX -var_dump($val > 0); // positive +var_dump($val > 0); // Valid values still work after clamping ini_set("session.cookie_lifetime", 200);