ext/session: do not reuse session state a save handler tore down - #22926
Open
iliaal wants to merge 1 commit into
Open
ext/session: do not reuse session state a save handler tore down#22926iliaal wants to merge 1 commit into
iliaal wants to merge 1 commit into
Conversation
session_regenerate_id() runs the userland write or destroy handler and then the close handler, and keeps operating on PS(id) afterwards. Any of them may call session_destroy(): php_session_destroy() runs php_rshutdown_session_globals() even when the recursive call was rejected, releasing PS(id) and setting it to NULL, so the following zend_string_release_ex() dereferences NULL. Check that the session is still active once every handler has run. Closes phpGH-22926
iliaal
force-pushed
the
fix/session-regenerate-id-handler-teardown
branch
from
July 29, 2026 18:14
085fdb1 to
127ea1e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
session_regenerate_id()runs the userland write or destroy handler, then the close handler, and keeps operating onPS(id)afterwards. Any of the three can callsession_destroy(), andphp_session_destroy()reachesphp_rshutdown_session_globals()even when the recursive call was rejected, so PS(id) is released and set to NULL before control returns andzend_string_release_ex()dereferences it.Smallest reproducer is a SessionHandler subclass whose
destroy()calls session_destroy(), followed bysession_regenerate_id(true): SIGSEGV on 8.4, 8.5 and master. The check now sits after every handler has run, so the function warns and returns false with the session left closed. I swept the nine SessionHandler methods against fourteen session functions, 126 combinations with each method calling session_destroy() once. session_regenerate_id() was the only function that crashed, from the write, destroy and close handlers, and nothing in the sweep crashes after this change. The two tests jorgsowa adds in #22687 cover write() and destroy() returning false and still pass here.