Fix GH-9618: drain __wakeup/__unserialize queue on unserialize failure#21716
Open
iliaal wants to merge 1 commit intophp:masterfrom
Open
Fix GH-9618: drain __wakeup/__unserialize queue on unserialize failure#21716iliaal wants to merge 1 commit intophp:masterfrom
iliaal wants to merge 1 commit intophp:masterfrom
Conversation
…lure `unserialize()` defers `__wakeup` and `__unserialize` calls until the end of parsing and drains them inside `PHP_VAR_UNSERIALIZE_DESTROY`. On the failure path, `zval_ptr_dtor(return_value)` in `php_unserialize_with_options` runs first and triggers destructors of the partially-built return value. A destructor that touches a sibling object sees that sibling in its pre-wakeup state, bypassing any invariant a `__wakeup` or `__unserialize` installed. Drain the deferred-call queue on the failure path before `zval_ptr_dtor` so `__wakeup` and `__unserialize` run first. Factor the per-slot drain out of `var_destroy()` into a static `var_drain_entry()` helper. Both `var_destroy()` and the new `var_invoke_delayed_calls()` share one implementation. Closes phpGH-9618
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.
Fixes #9618
unserialize()defers__wakeup()and__unserialize()calls until the entire payload has been parsed, then drains them insidePHP_VAR_UNSERIALIZE_DESTROYat the end ofphp_unserialize_with_options. On the failure path,zval_ptr_dtor(return_value)atext/standard/var.c:1469runs first and triggers destructors of the partially-built return value. A destructor that touches a sibling object sees that sibling in its pre-wakeup state, bypassing any invariant installed in__wakeup()or__unserialize().The POC uses a malformed property key length to abort unserialization after constructing both
A(with a destructor) andB(whose__wakeup()overwrites a field thatA::__destructlater passes toeval()). Before this fix,A::__destructruns first, reachesBwith its pre-wakeup value, andeval()executes the attacker-controlled string.Drain the deferred-call queue on the failure path, before the
zval_ptr_dtor(return_value)that triggers destructors.__wakeup()and__unserialize()now run first, so sibling destructors see the post-wakeup state.Implementation:
var_drain_entry()invar_unserializer.refactors the per-slot drain out ofvar_destroy(). Bothvar_destroy()and the newvar_invoke_delayed_calls()share one implementation.var_invoke_delayed_calls()walks the var dtor hash and callsvar_drain_entry()on each slot, clearing theVAR_WAKEUP_FLAG/VAR_UNSERIALIZE_FLAGmarkers so a subsequentvar_destroy()skips already-drained entries.php_unserialize_with_optionscallsvar_invoke_delayed_calls()on the failure branch, beforezval_ptr_dtor(return_value).Success path is unchanged. Tests cover both the
__wakeupand__unserializedrain paths; both fail on unfixed master and pass after the fix.