ext/session: fix cookie_lifetime overflow

ext/session/session.c

@@ -706,11 +706,25 @@ 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;
+		entry->value = zend_long_to_str(maxcookie);
 		return SUCCESS;
 	}
 

ext/session/tests/gh16290.phpt

@@ -6,8 +6,11 @@ session
 <?php include('skipif.inc'); ?>
 --FILE--
 <?php
+ob_start();
 session_set_cookie_params(PHP_INT_MAX, '/', null, false, true);
 echo "DONE";
+ob_end_flush();
 ?>
---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

ext/session/tests/session_cookie_lifetime_invalid.phpt

@@ -0,0 +1,43 @@
+--TEST--
+session.cookie_lifetime rejects non-integer values
+--EXTENSIONS--
+session
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+ini_set("session.cookie_lifetime", 100);
+
+// Float strings are rejected
+ini_set("session.cookie_lifetime", "1.5");
+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"));
+
+// Negative values are rejected
+ini_set("session.cookie_lifetime", -1);
+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"));
+
+ob_end_flush();
+?>
+--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"

ext/session/tests/session_cookie_lifetime_overflow.phpt

@@ -0,0 +1,34 @@
+--TEST--
+session.cookie_lifetime overflow value is clamped
+--EXTENSIONS--
+session
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+ob_start();
+
+// Set a valid value first
+ini_set("session.cookie_lifetime", 100);
+var_dump(ini_get("session.cookie_lifetime"));
+
+// Set an overflow value - should succeed with warning, value clamped
+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);
+
+// 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"

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"]=>

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();

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