From 844856c2f28e080666d22173342ddd9f23e6384e Mon Sep 17 00:00:00 2001 From: Jorg Sowa Date: Sat, 29 Jun 2024 21:55:44 +0200 Subject: [PATCH] Check session_create_id() input for null byte --- ext/session/session.c | 12 +++---- .../tests/session_create_id_basic.phpt | 8 +++++ .../session_create_id_invalid_prefix.phpt | 31 +++++++++++++++++++ 3 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 ext/session/tests/session_create_id_invalid_prefix.phpt diff --git a/ext/session/session.c b/ext/session/session.c index 1203c3baf162c..e86aeded60d05 100644 --- a/ext/session/session.c +++ b/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(); } diff --git a/ext/session/tests/session_create_id_basic.phpt b/ext/session/tests/session_create_id_basic.phpt index 58d61100fdada..089d366c20176 100644 --- a/ext/session/tests/session_create_id_basic.phpt +++ b/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) "" diff --git a/ext/session/tests/session_create_id_invalid_prefix.phpt b/ext/session/tests/session_create_id_invalid_prefix.phpt new file mode 100644 index 0000000000000..0a4e2c2d40013 --- /dev/null +++ b/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-- + +--FILE-- + +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