Skip to content

ext/session: validate session id just before sending cookie - #25082

Draft
Sjord wants to merge 1 commit into
php:masterfrom
Sjord:validate-outgoing-session-cookie
Draft

Sjord wants to merge 1 commit into
php:masterfrom
Sjord:validate-outgoing-session-cookie

Conversation

@Sjord

@Sjord Sjord commented Oct 31, 2026

Copy link
Copy Markdown
Contributor

Validate the session ID so we don't send an invalid cookie. This is meant as a hardening measure. In php2shell memory was dumped through the session cookie, making it possible to bypass ASLR. By validating the cookie value we can prevent this.

Related to #24997

Validate the session ID so we don't send an invalid cookie. This is
meant as a hardening measure. In php2shell memory was dumped through the
session cookie, making it possible to bypass ASLR. By validating the
cookie value we can prevent this.

Related to php#24997
@Sjord

Sjord commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

I don't like how it returns SUCCESS even if the cookie is invalid. If it returns FAILURE it causes quite a few tests to fail, because it aborts the session setup process.

Details ``` GH-23043 (broken session id code can cause zend_mm_heap corrupted) [ext/session/tests/user_session_module/gh23043.phpt] Test session_set_save_handler() function: class with create_sid [ext/session/tests/user_session_module/session_set_save_handler_class_017.phpt] Test session_set_save_handler() function: class with validate_sid [ext/session/tests/user_session_module/session_set_save_handler_class_018.phpt] session_set_save_handler(): optional closures not set again in second call [ext/session/tests/user_session_module/session_set_save_handler_multiple.phpt] Test session_set_save_handler() function: id interface [ext/session/tests/user_session_module/session_set_save_handler_iface_003.phpt] Test session_set_save_handler() function: create_sid [ext/session/tests/user_session_module/session_set_save_handler_sid_001.phpt] ```

Ping @arnaud-lb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant