diff --git a/ext/standard/php_var.h b/ext/standard/php_var.h index 8839a2f3df73..d131a4a88b85 100644 --- a/ext/standard/php_var.h +++ b/ext/standard/php_var.h @@ -62,6 +62,8 @@ 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 zend_long var_delayed_calls_mark(php_unserialize_data_t *var_hash); +PHPAPI void var_invoke_delayed_calls(php_unserialize_data_t *var_hash, zend_long from); 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 000000000000..19eba4d3263a --- /dev/null +++ b/ext/standard/tests/serialize/gh9618.phpt @@ -0,0 +1,40 @@ +--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"; + } +} + +$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)); +?> +--EXPECT-- +B::__wakeup +B::__call end='wakeup-guard' +bool(false) diff --git a/ext/standard/tests/serialize/gh9618_nested.phpt b/ext/standard/tests/serialize/gh9618_nested.phpt new file mode 100644 index 000000000000..871f374c53d5 --- /dev/null +++ b/ext/standard/tests/serialize/gh9618_nested.phpt @@ -0,0 +1,43 @@ +--TEST-- +GH-9618 (a failing nested unserialize() must not drain the outer call's queue) +--FILE-- + +--EXPECT-- +B::unserialize done +A::__wakeup +bool(true) diff --git a/ext/standard/tests/serialize/gh9618_nested_bypass.phpt b/ext/standard/tests/serialize/gh9618_nested_bypass.phpt new file mode 100644 index 000000000000..76679253227b --- /dev/null +++ b/ext/standard/tests/serialize/gh9618_nested_bypass.phpt @@ -0,0 +1,62 @@ +--TEST-- +GH-9618 (__wakeup runs before sibling destructors when the failure is nested) +--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"; + } +} + +class W implements Serializable +{ + public function serialize(): string + { + return 'x'; + } + + public function unserialize($data): void + { + @unserialize('O:1:"A":2:{s:4:"info";O:1:"B":1:{s:3:"end";N;}s:6:"Aend";s:1:"1";}'); + echo "W::unserialize done\n"; + } + + public function __serialize(): array + { + return []; + } + + public function __unserialize(array $data): void + { + } +} + +var_dump(@unserialize('C:1:"W":1:{x}') !== false); +?> +--EXPECT-- +B::__wakeup +W::unserialize done +B::__call end='wakeup-guard' +bool(true) diff --git a/ext/standard/tests/serialize/gh9618_unserialize.phpt b/ext/standard/tests/serialize/gh9618_unserialize.phpt new file mode 100644 index 000000000000..9811d5eb8162 --- /dev/null +++ b/ext/standard/tests/serialize/gh9618_unserialize.phpt @@ -0,0 +1,45 @@ +--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"; + } +} + +$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 f94c1cf09584..14811e65120e 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -1409,7 +1409,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co php_unserialize_data_t var_hash; zval *retval; HashTable *class_hash = NULL, *prev_class_hash; - zend_long prev_max_depth, prev_cur_depth; + zend_long prev_max_depth, prev_cur_depth, delayed_calls_mark; if (buf_len == 0) { RETURN_FALSE; @@ -1480,6 +1480,7 @@ PHPAPI void php_unserialize_with_options(zval *return_value, const char *buf, co } } + delayed_calls_mark = var_delayed_calls_mark(&var_hash); if (BG(unserialize).level > 1) { retval = var_tmp_var(&var_hash); } else { @@ -1490,6 +1491,7 @@ 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); } + var_invoke_delayed_calls(&var_hash, delayed_calls_mark); 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 1fb8793c2bbc..3b27fd58b604 100644 --- a/ext/standard/var_unserializer.re +++ b/ext/standard/var_unserializer.re @@ -53,6 +53,7 @@ struct php_unserialize_data { HashTable *ref_props; zend_long cur_depth; zend_long max_depth; + zend_long dtor_slots; var_entries entries; }; @@ -67,6 +68,7 @@ PHPAPI php_unserialize_data_t php_var_unserialize_init(void) { d->ref_props = NULL; d->cur_depth = 0; d->max_depth = BG(unserialize_max_depth); + d->dtor_slots = 0; d->entries.used_slots = 0; d->entries.next = NULL; if (!BG(serialize_lock)) { @@ -191,6 +193,7 @@ static zend_always_inline zval *tmp_var(php_unserialize_data_t *var_hashx, zend_ ZVAL_UNDEF(&var_hash->data[var_hash->used_slots]); Z_EXTRA(var_hash->data[var_hash->used_slots]) = 0; } + (*var_hashx)->dtor_slots += num; return &var_hash->data[used_slots]; } @@ -237,6 +240,90 @@ static zval *var_access(php_unserialize_data_t *var_hashx, zend_long id) return var_hash->data[id]; } +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) { + /* Perform delayed __wakeup calls */ + 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) { + /* Perform delayed __unserialize calls */ + Z_EXTRA_P(zv) = 0; + if (!*delayed_call_failed) { + zval param; + ZVAL_COPY(¶m, &var_dtor_hash->data[i + 1]); + + zend_object_set_properties_reinitable(Z_OBJ_P(zv), /* reinitable */ true); + 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)--; + zend_object_set_properties_reinitable(Z_OBJ_P(zv), /* reinitable */ false); + zval_ptr_dtor(¶m); + } else { + GC_ADD_FLAGS(Z_OBJ_P(zv), IS_OBJ_DESTRUCTOR_CALLED); + } + } +} + +PHPAPI zend_long var_delayed_calls_mark(php_unserialize_data_t *var_hashx) +{ + return (*var_hashx)->dtor_slots; +} + +PHPAPI void var_invoke_delayed_calls(php_unserialize_data_t *var_hashx, zend_long from) +{ + var_dtor_entries *var_dtor_hash = (*var_hashx)->first_dtor; + bool delayed_call_failed = 0; + zend_long seen = 0; + + while (var_dtor_hash) { + if (seen + var_dtor_hash->used_slots > from) { + for (zend_long i = from > seen ? from - seen : 0; i < var_dtor_hash->used_slots; i++) { + var_drain_entry(var_dtor_hash, i, &delayed_call_failed); + } + } + seen += var_dtor_hash->used_slots; + var_dtor_hash = var_dtor_hash->next; + } +} + PHPAPI void var_destroy(php_unserialize_data_t *var_hashx) { void *next; @@ -257,67 +344,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]); - - zend_object_set_properties_reinitable(Z_OBJ_P(zv), /* reinitable */ true); - 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)--; - zend_object_set_properties_reinitable(Z_OBJ_P(zv), /* reinitable */ false); - 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));