validation for cookie lifetime

Author: jorgsowa

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;

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")); // unchanged
+
+// Non-numeric strings are rejected
+ini_set("session.cookie_lifetime", "abc");
+var_dump(ini_get("session.cookie_lifetime")); // unchanged
+
+// Negative values are rejected
+ini_set("session.cookie_lifetime", -1);
+var_dump(ini_get("session.cookie_lifetime")); // unchanged
+
+// Negative overflow strings are rejected
+ini_set("session.cookie_lifetime", "-99999999999999999999");
+var_dump(ini_get("session.cookie_lifetime")); // unchanged
+
+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_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