Repository navigation
Conversation
php_var_unserialize_init() joined the context of any parse in progress, so user code that runs mid-parse and calls unserialize() resolved back-references into the outer value, including values being destroyed: a destructor run when an __unserialize() payload fails to parse or by the garbage collector, an error handler for a parser warning, an allowed_classes __toString(), a session_decode() overwrite. Only the unserialize handler of a C: payload is meant to share it. object_custom() now publishes the context for the duration of that call, a nested init shares it only while serialize_lock is not held, as serialize() does, and a parse hides it for its own duration. Fiber switches are blocked and a bailout restores the previous context while it is published. A nested init under serialize_lock, as in a C: payload unserialized from __wakeup() or __unserialize(), now keeps the allowed_classes and max_depth of the payload. Fixes phpGH-24674
Owner
Author
|
Promoted upstream: php#24751 |
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
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.
php_var_unserialize_init() hands the context of a parse in progress to any caller, so user code that runs mid-parse and calls unserialize() (a destructor from a failed __unserialize() payload or the GC, an error handler, an allowed_classes __toString(), a session_decode() overwrite) resolves R: and r: into the outer value, including values being destroyed.
The context is now shared only with the unserialize handler of a C: payload, and only while serialize_lock is not held, as serialize() does; under the lock the nested call gets its own context with the same allowed_classes and max_depth, which also closes an allowed_classes bypass from __wakeup(). Fiber switches inside that handler now throw FiberError, so this targets master; it applies to PHP-8.4 if a backport is wanted.
php#24433 fixes the reported __unserialize() case on PHP-8.4. php#24551, php#24693 and php#24538 change var_destroy(), where 08f4b64 already fixed phpGH-24481.