From d2c705267e35e72714e088921ab11ea54b5f5beb Mon Sep 17 00:00:00 2001
From: David Carlier <devnexen@gmail.com>
Date: Mon, 6 Jan 2025 20:25:48 +0000
Subject: [PATCH] ext/session: session_start() options arguments type checks.

---
 ext/session/session.c                         | 57 +++++++++++--------
 ext/session/tests/session_start_error.phpt    |  9 +++
 .../tests/session_start_read_and_close.phpt   | 18 ++++--
 3 files changed, 56 insertions(+), 28 deletions(-)

diff --git a/ext/session/session.c b/ext/session/session.c
index 523e6f8429902..55a2265df7be9 100644
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -2634,9 +2634,8 @@ PHP_FUNCTION(session_start)
 {
 	zval *options = NULL;
 	zval *value;
-	zend_ulong num_idx;
 	zend_string *str_idx;
-	zend_long read_and_close = 0;
+	bool read_and_close = false;
 
 	if (zend_parse_parameters(ZEND_NUM_ARGS(), "|a", &options) == FAILURE) {
 		RETURN_THROWS();
@@ -2659,32 +2658,44 @@ PHP_FUNCTION(session_start)
 
 	/* set options */
 	if (options) {
-		ZEND_HASH_FOREACH_KEY_VAL(Z_ARRVAL_P(options), num_idx, str_idx, value) {
-			if (str_idx) {
-				switch(Z_TYPE_P(value)) {
-					case IS_STRING:
-					case IS_TRUE:
-					case IS_FALSE:
-					case IS_LONG:
-						if (zend_string_equals_literal(str_idx, "read_and_close")) {
-							read_and_close = zval_get_long(value);
+		ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(options), str_idx, value) {
+			if (UNEXPECTED(!str_idx)) {
+				zend_argument_value_error(1, "must be of type array with keys as string");
+				RETURN_THROWS();
+			}
+			switch(Z_TYPE_P(value)) {
+				case IS_STRING:
+				case IS_TRUE:
+				case IS_FALSE:
+				case IS_LONG:
+					if (zend_string_equals_literal(str_idx, "read_and_close")) {
+						zend_long tmp;
+						if (Z_TYPE_P(value) != IS_STRING) {
+							tmp = zval_get_long(value);
 						} else {
-							zend_string *tmp_val;
-							zend_string *val = zval_get_tmp_string(value, &tmp_val);
-							if (php_session_start_set_ini(str_idx, val) == FAILURE) {
-								php_error_docref(NULL, E_WARNING, "Setting option \"%s\" failed", ZSTR_VAL(str_idx));
+							if (is_numeric_string(Z_STRVAL_P(value), Z_STRLEN_P(value), &tmp, NULL, false) != IS_LONG) {
+								zend_type_error("%s(): Option \"%s\" value must be of type compatible with int, \"%s\" given",
+										get_active_function_name(), ZSTR_VAL(str_idx), Z_STRVAL_P(value)
+									       );
+								RETURN_THROWS();
 							}
-							zend_tmp_string_release(tmp_val);
 						}
-						break;
-					default:
-						zend_type_error("%s(): Option \"%s\" must be of type string|int|bool, %s given",
+						read_and_close = (tmp > 0);
+					} else {
+						zend_string *tmp_val;
+						zend_string *val = zval_get_tmp_string(value, &tmp_val);
+						if (php_session_start_set_ini(str_idx, val) == FAILURE) {
+							php_error_docref(NULL, E_WARNING, "Setting option \"%s\" failed", ZSTR_VAL(str_idx));
+						}
+						zend_tmp_string_release(tmp_val);
+					}
+					break;
+				default:
+					zend_type_error("%s(): Option \"%s\" must be of type string|int|bool, %s given",
 							get_active_function_name(), ZSTR_VAL(str_idx), zend_zval_value_name(value)
-						);
-						RETURN_THROWS();
-				}
+						       );
+					RETURN_THROWS();
 			}
-			(void) num_idx;
 		} ZEND_HASH_FOREACH_END();
 	}
 
diff --git a/ext/session/tests/session_start_error.phpt b/ext/session/tests/session_start_error.phpt
index 741a6bd34e99a..f616b8d4e95da 100644
--- a/ext/session/tests/session_start_error.phpt
+++ b/ext/session/tests/session_start_error.phpt
@@ -15,6 +15,14 @@ try {
     echo $exception->getMessage() . "\n";
 }
 
+$read_and_close = "false";
+
+try {
+    session_start([$read_and_close]);
+} catch (ValueError $exception) {
+    echo $exception::class, ': ', $exception->getMessage() . "\n";
+}
+
 var_dump(session_start(['option' => false]));
 
 ob_end_flush();
@@ -22,6 +30,7 @@ ob_end_flush();
 ?>
 --EXPECTF--
 session_start(): Option "option" must be of type string|int|bool, stdClass given
+ValueError: session_start(): Argument #1 ($options) must be of type array with keys as string
 
 Warning: session_start(): Setting option "option" failed in %s on line %d
 bool(true)
diff --git a/ext/session/tests/session_start_read_and_close.phpt b/ext/session/tests/session_start_read_and_close.phpt
index aee8f1e76b04c..48ebfea636455 100644
--- a/ext/session/tests/session_start_read_and_close.phpt
+++ b/ext/session/tests/session_start_read_and_close.phpt
@@ -19,14 +19,18 @@ foreach ($valuesEnablingReadAndClose as $value) {
 }
 
 foreach ($valuesDisablingReadAndClose as $value) {
-    session_start(["read_and_close" => $value]);
+    try {
+    	session_start(["read_and_close" => $value]);
+    } catch (TypeError $e) {
+    	echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+    }
     var_dump(session_status() === PHP_SESSION_ACTIVE);
     session_write_close();
 }
 
 try {
     session_start(["read_and_close" => 1.0]);
-} catch (Throwable $e) {
+} catch (TypeError $e) {
     echo $e::class, ': ', $e->getMessage(), PHP_EOL;
 }
 
@@ -38,12 +42,16 @@ bool(true)
 bool(true)
 bool(true)
 bool(true)
+bool(false)
+
+Notice: session_start(): Ignoring session_start() because a session is already active (started from %s on line %d) in %s on line %d
 bool(true)
 bool(true)
+TypeError: session_start(): Option "read_and_close" value must be of type compatible with int, "false" given
+bool(false)
 bool(true)
 bool(true)
-bool(true)
-bool(true)
-bool(true)
+TypeError: session_start(): Option "read_and_close" value must be of type compatible with int, "no" given
+bool(false)
 bool(true)
 TypeError: session_start(): Option "read_and_close" must be of type string|int|bool, float given