diff --git a/ext/session/session.c b/ext/session/session.c index 702b56020ae9..a639eca7cd39 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1493,6 +1493,15 @@ static zend_result php_session_send_cookie(void) ZEND_ASSERT(strpbrk(ZSTR_VAL(PS(session_name)), SESSION_FORBIDDEN_CHARS) == NULL); + /* Validate the session ID again right before it is sent. The ID may have + * been corrupted in memory after it was initially validated, and sending + * it in a cookie could leak the corrupted contents. Do not fail the + * session here, as user save handlers may use IDs outside the default + * charset. */ + if (UNEXPECTED(php_session_valid_key_ex(PS(id)) == FAILURE)) { + return SUCCESS; + } + /* URL encode id because it might be user supplied */ e_id = php_url_encode(ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id))); diff --git a/ext/session/tests/session_id_invalid_cookie.phpt b/ext/session/tests/session_id_invalid_cookie.phpt new file mode 100644 index 000000000000..b852956a2cd8 --- /dev/null +++ b/ext/session/tests/session_id_invalid_cookie.phpt @@ -0,0 +1,25 @@ +--TEST-- +Invalid session IDs are not sent in the session cookie +--EXTENSIONS-- +session +--INI-- +session.save_handler=files +session.save_path= +session.use_cookies=1 +session.use_strict_mode=0 +--CGI-- +--FILE-- + +--EXPECTF-- +Warning: session_start(): Session ID is too long or contains illegal characters. Only the A-Z, a-z, 0-9, "-", and "," characters are allowed in %s on line %d + +Warning: session_start(): Failed to read session data: files (path: ) in %s on line %d +bool(false)