Check session_create_id() input for null byte (php#14728)

Author: jorgsowa

ext/session/session.c

@@ -360,17 +360,15 @@ PHPAPI zend_result php_session_valid_key(const char *key) /* {{{ */
 	size_t len;
 	const char *p;
 	char c;
-	zend_result ret = SUCCESS;
 
 	for (p = key; (c = *p); p++) {
-		/* valid characters are a..z,A..Z,0..9 */
+		/* valid characters are [a-z], [A-Z], [0-9], - (hyphen) and , (comma) */
 		if (!((c >= 'a' && c <= 'z')
 				|| (c >= 'A' && c <= 'Z')
 				|| (c >= '0' && c <= '9')
 				|| c == ','
 				|| c == '-')) {
-			ret = FAILURE;
-			break;
+			return FAILURE;
 		}
 	}
 
@@ -379,10 +377,10 @@ PHPAPI zend_result php_session_valid_key(const char *key) /* {{{ */
 	/* Somewhat arbitrary length limit here, but should be way more than
 	   anyone needs and avoids file-level warnings later on if we exceed MAX_PATH */
 	if (len == 0 || len > PS_MAX_SID_LENGTH) {
-		ret = FAILURE;
+		return FAILURE;
 	}
 
-	return ret;
+	return SUCCESS;
 }
 /* }}} */
 
@@ -2374,7 +2372,7 @@ PHP_FUNCTION(session_create_id)
 	zend_string *prefix = NULL, *new_id;
 	smart_str id = {0};
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "|S", &prefix) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "|P", &prefix) == FAILURE) {
 		RETURN_THROWS();
 	}
 

ext/session/tests/session_create_id_basic.phpt

@@ -16,6 +16,10 @@ echo "*** Testing session_create_id() : basic functionality ***\n";
 
 // No session
 var_dump(session_create_id());
+var_dump(session_create_id(''));
+var_dump(session_create_id(','));
+var_dump(session_create_id('-'));
+var_dump(session_create_id('0123456789'));
 var_dump(session_create_id('ABCD'));
 
 ini_set('session.use_strict_mode', true);
@@ -42,6 +46,10 @@ ob_end_flush();
 --EXPECTF--
 *** Testing session_create_id() : basic functionality ***
 string(32) "%s"
+string(32) "%s"
+string(33) ",%s"
+string(33) "-%s"
+string(42) "0123456789%s"
 string(36) "ABCD%s"
 string(35) "XYZ%s"
 string(0) ""

ext/session/tests/session_create_id_invalid_prefix.phpt

@@ -0,0 +1,31 @@
+--TEST--
+Test session_create_id() function : invalid prefix
+--INI--
+session.save_handler=files
+session.sid_length=32
+--EXTENSIONS--
+session
+--SKIPIF--
+<?php include('skipif.inc'); ?>
+--FILE--
+<?php
+
+var_dump(session_create_id('_'));
+var_dump(session_create_id('%'));
+var_dump(session_create_id("AB\0CD"));
+
+
+?>
+Done
+--EXPECTF--
+Warning: session_create_id(): Prefix cannot contain special characters. Only the A-Z, a-z, 0-9, "-", and "," characters are allowed in %s on line %d
+bool(false)
+
+Warning: session_create_id(): Prefix cannot contain special characters. Only the A-Z, a-z, 0-9, "-", and "," characters are allowed in %s on line %d
+bool(false)
+
+Fatal error: Uncaught ValueError: session_create_id(): Argument #1 ($prefix) must not contain any null bytes in %s:%d
+Stack trace:
+#0 %s(5): session_create_id('AB\x00CD')
+#1 {main}
+  thrown in %s