From cb0e4b3cf3ed9b3063481a824430d45236d8136c Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Tue, 29 Sep 2026 18:12:18 +0200 Subject: [PATCH] Fix deepclone_from_array() issues found by differential fuzzing When the array of a reference holds a & to a reference resolved after it, that second reference is already bound when its marker is resolved: dc_resolve() now dereferences the values it gets, instead of rejecting them as not being of type int, array or string. Serialized objects whose data references them back, eg R:1;, unserialize to a reference, which is now unwrapped instead of being rejected. When a delayed __unserialize() or __wakeup() call throws while unserializing one, the call fails right away instead of leaking the objects created before. Typed properties are written like unserialize() and the polyfill do: null on a non-nullable one and a scalar on one typed with a backed enum throw a TypeError, instead of the conveniences of deepclone_hydrate(). --- CHANGELOG.md | 11 ++++ deepclone.c | 23 ++++++++- ...epclone_from_array_refs_to_later_refs.phpt | 51 +++++++++++++++++++ ...epclone_from_array_serialized_classes.phpt | 41 +++++++++++++++ ...deepclone_from_array_typed_properties.phpt | 32 ++++++++++++ 5 files changed, 156 insertions(+), 2 deletions(-) create mode 100644 tests/deepclone_from_array_refs_to_later_refs.phpt create mode 100644 tests/deepclone_from_array_serialized_classes.phpt create mode 100644 tests/deepclone_from_array_typed_properties.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index 46e808b..81339aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - References that nothing else holds, eg after `unset()` of the other side, are exported as values, like `serialize()` and the polyfill do, instead of taking a reference id. +- `deepclone_from_array()` rejected payloads that `deepclone_to_array()` + produced when a reference holds an array that holds a `&` to a reference + holding an object, a closure, an enum or an array, eg `$v = [&$arr, &$arr, + &$x]` with `$arr = ['x' => &$x]`. +- `deepclone_from_array()` rejected `Serializable` objects whose data + references them back, eg `R:1;`, and leaked the objects already created when + a nested `__unserialize()` threw. +- `deepclone_from_array()` writes typed properties like `unserialize()` and + the polyfill do: `null` on a non-nullable one and a scalar on one typed with + a backed enum throw a `TypeError` instead of leaving it uninitialized or + casting the scalar, which `deepclone_hydrate()` still does. ## [0.8.6] - 2026-09-29 diff --git a/deepclone.c b/deepclone.c index 480569e..96e9792 100644 --- a/deepclone.c +++ b/deepclone.c @@ -219,6 +219,9 @@ static zend_always_inline void dc_create_fake_closure(zval *res, zend_function * #define DEEPCLONE_HYDRATE_PRESERVE_REFS (1 << 2) #define DEEPCLONE_HYDRATE_FLAGS_MASK \ (DEEPCLONE_HYDRATE_CALL_HOOKS | DEEPCLONE_HYDRATE_NO_LAZY_INIT | DEEPCLONE_HYDRATE_PRESERVE_REFS) +/* Internal: write like unserialize() does, without the conveniences of + * deepclone_hydrate() for null and backed enums */ +#define DC_WRITE_LIKE_UNSERIALIZE (1 << 30) /* IS_PROP_REINITABLE (readonly clone-with bookkeeping) landed in PHP 8.3. * On 8.2 there is no such flag; clearing 0 bits is a no-op. */ @@ -1156,6 +1159,7 @@ static bool dc_write_backed_property(zend_object *obj, zend_property_info *pi, * Properties of internal classes are excluded too: their methods expect * them initialized, eg Exception::getTraceAsString() crashes otherwise. */ if (Z_TYPE_P(value) == IS_NULL + && !(flags & DC_WRITE_LIKE_UNSERIALIZE) && ZEND_TYPE_IS_SET(pi->type) && !ZEND_TYPE_ALLOW_NULL(pi->type) && !DC_PROP_HAS_HOOKS(pi) @@ -1181,6 +1185,7 @@ static bool dc_write_backed_property(zend_object *obj, zend_property_info *pi, zval enum_holder; bool enum_holder_used = false; if ((Z_TYPE_P(value) == IS_LONG || Z_TYPE_P(value) == IS_STRING) + && !(flags & DC_WRITE_LIKE_UNSERIALIZE) && ZEND_TYPE_HAS_NAME(pi->type) && !ZEND_TYPE_HAS_LIST(pi->type) /* Only cast for types of the form `Enum` or `?Enum` — unions like @@ -3869,6 +3874,10 @@ static ZEND_COLD zend_never_inline bool dc_resolve_unmatched_markers(HashTable * */ static void dc_resolve(zval *value, zval *mask, zval *objects, uint32_t num_objects, HashTable *refs, HashTable *allowed_set, zval *retval) { + /* A reference that a hard-ref marker bound to it, eg the value of a ref + * whose array holds an & to it and was resolved before it */ + ZVAL_DEREF(value); + if (EXPECTED(DC_MASK_IS_OBJ_REF(mask))) { if (UNEXPECTED(Z_TYPE_P(value) != IS_LONG)) { zend_value_error("deepclone_from_array(): malformed payload, object reference value must be of type int, %s given", zend_zval_value_name(value)); @@ -4642,7 +4651,7 @@ static void dc_lazy_hydrate(dc_lazy_ctx *ctx, zend_object *obj, uint32_t id) if (slot->pi) { EG(fake_scope) = slot->scope_ce != zend_standard_class_def ? slot->scope_ce : NULL; - bool ok = dc_write_backed_property(obj, slot->pi, slot->name, &final_val, 0); + bool ok = dc_write_backed_property(obj, slot->pi, slot->name, &final_val, DC_WRITE_LIKE_UNSERIALIZE); EG(fake_scope) = NULL; zval_ptr_dtor(&final_val); if (UNEXPECTED(!ok)) { @@ -5320,6 +5329,16 @@ PHP_FUNCTION(deepclone_from_array) DC_INVALID("deepclone_from_array(): Argument #1 ($data) failed to unserialize object %u", id); } PHP_VAR_UNSERIALIZE_DESTROY(var_hash); + if (UNEXPECTED(EG(exception))) { + /* Thrown by a delayed __unserialize() or __wakeup() call */ + zval_ptr_dtor(&obj_zval); + goto cleanup; + } + /* A back-reference to the root, eg R:1; in its data, makes it a + * reference */ + if (Z_ISREF(obj_zval)) { + zend_unwrap_reference(&obj_zval); + } /* The result is later dereferenced as a zend_object*; a malformed * payload can carry any serialize form (i:…, s:…, a:…), so reject * anything that did not decode to an object before storing it. */ @@ -5674,7 +5693,7 @@ PHP_FUNCTION(deepclone_from_array) if (dc_is_backed_declared_property(use_pi)) { /* deepclone_from_array always uses setRawValue semantics * (flags=0): payload-driven, same policy as unserialize(). */ - bool ok = dc_write_backed_property(obj, use_pi, prop_name, &final_val, 0); + bool ok = dc_write_backed_property(obj, use_pi, prop_name, &final_val, DC_WRITE_LIKE_UNSERIALIZE); zval_ptr_dtor(&final_val); if (UNEXPECTED(!ok)) { EG(fake_scope) = old_scope; diff --git a/tests/deepclone_from_array_refs_to_later_refs.phpt b/tests/deepclone_from_array_refs_to_later_refs.phpt new file mode 100644 index 0000000..a2a0c1c --- /dev/null +++ b/tests/deepclone_from_array_refs_to_later_refs.phpt @@ -0,0 +1,51 @@ +--TEST-- +deepclone_from_array() resolves references whose value holds a reference resolved after them +--EXTENSIONS-- +deepclone +--FILE-- + &$x]; +$v = [&$arr, &$arr, &$x]; +$x = new stdClass(); +unset($arr); +check($v); + +$x = null; +$arr = ['x' => &$x]; +$v = [&$arr, &$arr, &$x]; +$x = strlen(...); +unset($arr); +check($v); + +$x = null; +$arr = ['x' => &$x]; +$v = [&$arr, &$arr, &$x]; +$x = [new stdClass()]; +unset($arr); +check($v); + +// An array holding a reference to itself +$y = [1]; +$y['me'] = &$y; +$x = [$y]; +$x[1] = &$x[0]; +$c = deepclone_from_array(deepclone_to_array($x)); +var_dump($c[1]['me']['me'][0]); +?> +--EXPECT-- +string(7) "changed" +string(7) "changed" +string(7) "changed" +string(7) "changed" +string(7) "changed" +string(7) "changed" +int(1) diff --git a/tests/deepclone_from_array_serialized_classes.phpt b/tests/deepclone_from_array_serialized_classes.phpt new file mode 100644 index 0000000..a5520b7 --- /dev/null +++ b/tests/deepclone_from_array_serialized_classes.phpt @@ -0,0 +1,41 @@ +--TEST-- +deepclone_from_array() handles serialized objects that reference themselves or throw +--EXTENSIONS-- +deepclone +--FILE-- +x); } + public function unserialize($data): void { $this->x = unserialize($data); } +}'); + +class SerRef +{ + public $a; + public function __serialize(): array { return ['a' => &$this->a]; } + public function __unserialize(array $data): void { $this->a = &$data['a']; } +} + +// The payload holds C:6:"OldSer":...{...R:1;}, which unserializes to a reference +$o = new OldSer(); +$o->x = new SerRef(); +$o->x->a = $o; +$c = deepclone_from_array(deepclone_to_array($o)); +var_dump($c->x->a === $c); + +class UnThrow +{ + public function __unserialize(array $data): void { throw new DomainException('boom'); } +} + +try { + deepclone_from_array(['classes' => ['stdClass', 'O:7:"UnThrow":0:{}'], 'objectMeta' => [0, 1], 'prepared' => [0, 1], 'mask' => [true, true]]); +} catch (DomainException $e) { + echo $e->getMessage(), "\n"; +} +?> +--EXPECT-- +bool(true) +boom diff --git a/tests/deepclone_from_array_typed_properties.phpt b/tests/deepclone_from_array_typed_properties.phpt new file mode 100644 index 0000000..d0f2ba7 --- /dev/null +++ b/tests/deepclone_from_array_typed_properties.phpt @@ -0,0 +1,32 @@ +--TEST-- +deepclone_from_array() writes typed properties like unserialize(), without the conveniences of deepclone_hydrate() +--EXTENSIONS-- +deepclone +--FILE-- + [null]], ['s' => ['H']]] as $props) { + try { + deepclone_from_array(['classes' => 'Typed', 'objectMeta' => 1, 'prepared' => 0, 'properties' => ['stdClass' => $props]]); + } catch (TypeError $e) { + echo $e->getMessage(), "\n"; + } +} + +// deepclone_hydrate() leaves the property uninitialized and casts the scalar +$o = deepclone_hydrate('Typed', ['i' => null, 's' => 'H']); +var_dump(isset($o->i), $o->s); +?> +--EXPECT-- +Cannot assign null to property Typed::$i of type int +Cannot assign string to property Typed::$s of type Suit +bool(false) +enum(Suit::Hearts)