From 6c5407a494f50651cf2d666b3c3f0f1a7bdf1795 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 10 Apr 2026 15:43:03 -0400 Subject: [PATCH] Fix GH-9618: drain __wakeup/__unserialize queue on unserialize failure `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 GH-9618 --- ext/standard/php_var.h | 1 + ext/standard/tests/serialize/gh9618.phpt | 45 ++++++ .../tests/serialize/gh9618_unserialize.phpt | 49 ++++++ ext/standard/var.c | 4 + ext/standard/var_unserializer.re | 145 +++++++++++------- 5 files changed, 188 insertions(+), 56 deletions(-) create mode 100644 ext/standard/tests/serialize/gh9618.phpt create mode 100644 ext/standard/tests/serialize/gh9618_unserialize.phpt diff --git a/ext/standard/php_var.h b/ext/standard/php_var.h index d70bbfed814a0..d721281b82763 100644 --- a/ext/standard/php_var.h +++ b/ext/standard/php_var.h @@ -64,6 +64,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co PHPAPI void var_replace(php_unserialize_data_t *var_hash, zval *ozval, zval *nzval); PHPAPI void var_push_dtor(php_unserialize_data_t *var_hash, zval *val); PHPAPI zval *var_tmp_var(php_unserialize_data_t *var_hashx); +PHPAPI void var_invoke_delayed_calls(php_unserialize_data_t *var_hash); PHPAPI void var_destroy(php_unserialize_data_t *var_hash); #endif /* PHP_VAR_H */ diff --git a/ext/standard/tests/serialize/gh9618.phpt b/ext/standard/tests/serialize/gh9618.phpt new file mode 100644 index 0000000000000..8f1b49d95e7d4 --- /dev/null +++ b/ext/standard/tests/serialize/gh9618.phpt @@ -0,0 +1,45 @@ +--TEST-- +GH-9618 (unserialize __wakeup bypass via malformed payload) +--FILE-- +info)) { + $this->info->probe(); + } + } +} + +class B +{ + public $end; + + public function __wakeup() + { + $this->end = 'wakeup-guard'; + echo "B::__wakeup\n"; + } + + public function __call($method, $args) + { + echo "B::__call end=" . var_export($this->end, true) . "\n"; + } +} + +// Malformed payload: second property key length is 6 but the actual key is 4 bytes. +// The unserializer aborts partway through after constructing A and B. +// Before this fix, A::__destruct ran before B::__wakeup, letting the destructor +// reach B with an attacker-controlled $end (null). After the fix, B::__wakeup +// runs first so the guard value is in place when A::__destruct calls into B. +$payload = 'O:1:"A":2:{s:4:"info";O:1:"B":1:{s:3:"end";N;}s:6:"Aend";s:1:"1";}'; + +var_dump(@unserialize($payload)); +?> +--EXPECTF-- +B::__wakeup +B::__call end='wakeup-guard' +bool(false) diff --git a/ext/standard/tests/serialize/gh9618_unserialize.phpt b/ext/standard/tests/serialize/gh9618_unserialize.phpt new file mode 100644 index 0000000000000..c4eca7b503bdf --- /dev/null +++ b/ext/standard/tests/serialize/gh9618_unserialize.phpt @@ -0,0 +1,49 @@ +--TEST-- +GH-9618 (__unserialize drained before destructors on failure path) +--FILE-- +info)) { + $this->info->probe(); + } + } +} + +class C +{ + public $end; + + public function __unserialize(array $data): void + { + $this->end = 'unserialize-guard'; + echo "C::__unserialize\n"; + } + + public function __serialize(): array + { + return ['end' => $this->end]; + } + + public function __call($method, $args) + { + echo "C::__call end=" . var_export($this->end, true) . "\n"; + } +} + +// Same malformed-length trick: second property key length claimed to be 6 but +// the actual key is "Aend" (4 bytes). Unserializer aborts after building both +// A and C. C uses __unserialize instead of __wakeup, exercising the +// VAR_UNSERIALIZE_FLAG drain path. +$payload = 'O:1:"A":2:{s:4:"info";O:1:"C":1:{s:3:"end";N;}s:6:"Aend";s:1:"1";}'; + +var_dump(@unserialize($payload)); +?> +--EXPECT-- +C::__unserialize +C::__call end='unserialize-guard' +bool(false) diff --git a/ext/standard/var.c b/ext/standard/var.c index fc8b2f4f37848..f44df5af40c8b 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -1465,6 +1465,10 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co php_error_docref(NULL, E_WARNING, "Error at offset " ZEND_LONG_FMT " of %zd bytes", (zend_long)((char*)p - buf), buf_len); } + /* Drain queued __wakeup / __unserialize calls before destructors of the + * partially-built return value run, so __wakeup-based input validation + * applies before sibling objects observe half-built state. See GH-9618. */ + var_invoke_delayed_calls(&var_hash); if (BG(unserialize).level <= 1) { zval_ptr_dtor(return_value); } diff --git a/ext/standard/var_unserializer.re b/ext/standard/var_unserializer.re index 353c7086d4304..dca065eb693ac 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -227,6 +227,93 @@ static zval *var_access(php_unserialize_data_t *var_hashx, zend_long id) return var_hash->data[id]; } +/* Invoke the deferred __wakeup / __unserialize call queued on a single + * var_dtor_hash slot, if any. Clears the slot's Z_EXTRA marker so the same + * slot won't be processed twice if the var hash is walked again later. + * + * delayed_call_failed is shared state across a drain loop: once any call + * fails, subsequent slots are marked IS_OBJ_DESTRUCTOR_CALLED instead of + * invoked. */ +static void var_drain_entry(var_dtor_entries *var_dtor_hash, zend_long i, bool *delayed_call_failed) +{ + zval *zv = &var_dtor_hash->data[i]; + + if (Z_EXTRA_P(zv) == VAR_WAKEUP_FLAG) { + Z_EXTRA_P(zv) = 0; + if (!*delayed_call_failed) { + zval retval; + zend_fcall_info fci; + zend_fcall_info_cache fci_cache; + + ZEND_ASSERT(Z_TYPE_P(zv) == IS_OBJECT); + + fci.size = sizeof(fci); + fci.object = Z_OBJ_P(zv); + fci.retval = &retval; + fci.param_count = 0; + fci.params = NULL; + fci.named_params = NULL; + ZVAL_UNDEF(&fci.function_name); + + fci_cache.function_handler = zend_hash_find_ptr( + &fci.object->ce->function_table, ZSTR_KNOWN(ZEND_STR_WAKEUP)); + fci_cache.object = fci.object; + fci_cache.called_scope = fci.object->ce; + + BG(serialize_lock)++; + if (zend_call_function(&fci, &fci_cache) == FAILURE || Z_ISUNDEF(retval)) { + *delayed_call_failed = 1; + GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); + } + BG(serialize_lock)--; + + zval_ptr_dtor(&retval); + } else { + GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); + } + } else if (Z_EXTRA_P(zv) == VAR_UNSERIALIZE_FLAG) { + Z_EXTRA_P(zv) = 0; + if (!*delayed_call_failed) { + zval param; + ZVAL_COPY(¶m, &var_dtor_hash->data[i + 1]); + + BG(serialize_lock)++; + zend_call_known_instance_method_with_1_params( + Z_OBJCE_P(zv)->__unserialize, Z_OBJ_P(zv), NULL, ¶m); + if (EG(exception)) { + *delayed_call_failed = 1; + GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); + } + BG(serialize_lock)--; + zval_ptr_dtor(¶m); + } else { + GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); + } + } +} + +/* Drain queued __wakeup / __unserialize calls without freeing the var hash. + * Used on the failure path of unserialize() to ensure deferred wakeups run + * before any destructors of the partially-unserialized return value are + * invoked, so that __wakeup-based input validation can still apply its + * post-conditions before sibling objects observe the half-built state. + * + * After this returns, the VAR_WAKEUP_FLAG / VAR_UNSERIALIZE_FLAG markers are + * cleared on every entry that was processed, so a subsequent var_destroy() + * call will not re-invoke the same delayed calls. */ +PHPAPI void var_invoke_delayed_calls(php_unserialize_data_t *var_hashx) +{ + var_dtor_entries *var_dtor_hash = (*var_hashx)->first_dtor; + bool delayed_call_failed = 0; + + while (var_dtor_hash) { + for (zend_long i = 0; i < var_dtor_hash->used_slots; i++) { + var_drain_entry(var_dtor_hash, i, &delayed_call_failed); + } + var_dtor_hash = var_dtor_hash->next; + } +} + PHPAPI void var_destroy(php_unserialize_data_t *var_hashx) { void *next; @@ -247,65 +334,11 @@ PHPAPI void var_destroy(php_unserialize_data_t *var_hashx) while (var_dtor_hash) { for (i = 0; i < var_dtor_hash->used_slots; i++) { - zval *zv = &var_dtor_hash->data[i]; #if VAR_ENTRIES_DBG fprintf(stderr, "var_destroy dtor(%p, %ld)\n", &var_dtor_hash->data[i], Z_REFCOUNT_P(&var_dtor_hash->data[i])); #endif - - if (Z_EXTRA_P(zv) == VAR_WAKEUP_FLAG) { - /* Perform delayed __wakeup calls */ - if (!delayed_call_failed) { - zval retval; - zend_fcall_info fci; - zend_fcall_info_cache fci_cache; - - ZEND_ASSERT(Z_TYPE_P(zv) == IS_OBJECT); - - fci.size = sizeof(fci); - fci.object = Z_OBJ_P(zv); - fci.retval = &retval; - fci.param_count = 0; - fci.params = NULL; - fci.named_params = NULL; - ZVAL_UNDEF(&fci.function_name); - - fci_cache.function_handler = zend_hash_find_ptr( - &fci.object->ce->function_table, ZSTR_KNOWN(ZEND_STR_WAKEUP)); - fci_cache.object = fci.object; - fci_cache.called_scope = fci.object->ce; - - BG(serialize_lock)++; - if (zend_call_function(&fci, &fci_cache) == FAILURE || Z_ISUNDEF(retval)) { - delayed_call_failed = 1; - GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); - } - BG(serialize_lock)--; - - zval_ptr_dtor(&retval); - } else { - GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); - } - } else if (Z_EXTRA_P(zv) == VAR_UNSERIALIZE_FLAG) { - /* Perform delayed __unserialize calls */ - if (!delayed_call_failed) { - zval param; - ZVAL_COPY(¶m, &var_dtor_hash->data[i + 1]); - - BG(serialize_lock)++; - zend_call_known_instance_method_with_1_params( - Z_OBJCE_P(zv)->__unserialize, Z_OBJ_P(zv), NULL, ¶m); - if (EG(exception)) { - delayed_call_failed = 1; - GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); - } - BG(serialize_lock)--; - zval_ptr_dtor(¶m); - } else { - GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); - } - } - - i_zval_ptr_dtor(zv); + var_drain_entry(var_dtor_hash, i, &delayed_call_failed); + i_zval_ptr_dtor(&var_dtor_hash->data[i]); } next = var_dtor_hash->next; efree_size(var_dtor_hash, sizeof(var_dtor_entries));