Skip to content

ext/session: report a rejected session cookie header - #198

Closed
iliaal wants to merge 1 commit into
PHP-8.4from
fix/session-cookie-header-failure
Closed

ext/session: report a rejected session cookie header#198
iliaal wants to merge 1 commit into
PHP-8.4from
fix/session-cookie-header-failure

Conversation

@iliaal

@iliaal iliaal commented Jul 29, 2026

Copy link
Copy Markdown
Owner

php_session_send_cookie() dropped the result of sapi_add_header_ex() and always returned SUCCESS. When the SAPI refuses a Set-Cookie header containing CR or LF, the warning it emits can reach a userland error handler, and if that handler calls session_destroy() then php_session_reset_id() carries on and appends the released PS(id). Reproducer is a set_error_handler() that destroys the session, session_set_cookie_params(['path' => "/\r\nX: y"]), then session_start(): SIGSEGV on 8.4, 8.5 and master, under both CLI and CGI.

php_session_reset_id() has handled a FAILURE from the cookie call since 8.4, it just never saw one. The Set-Cookie header is otherwise built from validated parts: session.name is checked by OnUpdateName, the ID is URL encoded, and cookie_samesite is restricted to a fixed set, so cookie_path and cookie_domain are the only values that can carry CR or LF into the header today.

@iliaal
iliaal force-pushed the fix/session-cookie-header-failure branch from 0e73a00 to d6c9488 Compare July 29, 2026 16:47
php_session_send_cookie() discarded the result of sapi_add_header_ex()
and always returned SUCCESS. When the SAPI refuses a Set-Cookie header
carrying CR or LF, the warning it emits can reach a userland error
handler that calls session_destroy(), and php_session_reset_id() then
appends the released PS(id). Return what sapi_add_header_ex() reports so
the caller stops before touching session state again.

Closes phpGH-22923
@iliaal
iliaal force-pushed the fix/session-cookie-header-failure branch from d6c9488 to 4264db1 Compare July 29, 2026 18:02
@iliaal

iliaal commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Promoted upstream as php#22923.

@iliaal iliaal closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant