From f7b2c3c86ea97dc2b3cbab3bc08b4d3da4dd9cbd Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 27 Sep 2026 07:31:18 +0200 Subject: [PATCH] Reject classes that keep their state internally in deepclone_from_array() deepclone_to_array() and deepclone_hydrate() reject the internal classes that keep their state out of their properties and declare no serialization API, like Redis, Imagick or the AMQP and Relay classes, but deepclone_from_array() created them when a payload named them, eg one produced by the polyfill. unserialize() creates them that way too, without their state, and some crash PHP then: RedisCluster::acl() or ImagickDraw::affine() segfault on such objects, and a Relay\Table does when destroyed. The class lookup of deepclone_from_array() now applies the same rule, which the three functions share in one helper, and throws NotInstantiableException. User classes that extend such a class without declaring a serialization API are rejected too, eg a subclass of RedisCluster, which crashed the same way, in all three functions. That's one more flag test per class and call for user classes. IteratorIterator and the other internal iterators wrapping another one are rejected too, instead of being created like unserialize() does, and so are their user subclasses. --- CHANGELOG.md | 9 ++ README.md | 12 +-- deepclone.c | 68 ++++++++++----- .../deepclone_from_array_internal_state.phpt | 83 +++++++++++++++++++ tests/deepclone_uninstantiable.phpt | 12 +-- 5 files changed, 151 insertions(+), 33 deletions(-) create mode 100644 tests/deepclone_from_array_internal_state.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index e8aa15f..62a83b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `deepclone_from_array()` rejects markers of masks that match no value, and of `refMasks` that match no reference, like the polyfill does, instead of ignoring them. +- `deepclone_from_array()` rejects the internal classes that keep their state + out of their properties, like `deepclone_to_array()` and + `deepclone_hydrate()` do, instead of creating them without that state when a + payload names them: some, like `Relay\Table`, crash PHP that way. This + includes `IteratorIterator` and the other internal iterators wrapping + another one. +- The three functions reject the user classes that extend such internal + classes without declaring a serialization API, eg a subclass of + `RedisCluster`, which crashed PHP the same way. ## [0.8.5] - 2026-09-23 diff --git a/README.md b/README.md index 35ee82a..e9e731c 100644 --- a/README.md +++ b/README.md @@ -171,11 +171,13 @@ classes, interfaces, traits and enums, and throw `InvalidArgumentException`. Malformed input and classes missing from `$allowed_classes` throw `ValueError`. -`deepclone_to_array()` and `deepclone_hydrate()` also reject internal classes -that keep their state out of their properties and declare no serialization -API, eg `Redis` or `Imagick`: `unserialize()` and the polyfill create them -without that state, but some crash when used that way. Only -`MultipleIterator`, heaps before PHP 8.5 and the classes of the dom, xsl, +All three functions also reject internal classes that keep their state out of +their properties and declare no serialization API, eg `Redis` or `Imagick`, +and the user classes that extend them without declaring one: `unserialize()` +creates them without that state, but some crash when used, or even destroyed, +that way. The polyfill can't tell these classes apart and +rejects the ones of the extensions it knows, eg zip, redis, relay or imagick. +Only `MultipleIterator`, heaps before PHP 8.5 and the classes of the dom, xsl, mysqli and soap extensions, eg `DOMNodeList`, are created like `unserialize()` does. diff --git a/deepclone.c b/deepclone.c index f45cc31..7009ab5 100644 --- a/deepclone.c +++ b/deepclone.c @@ -428,6 +428,36 @@ static bool dc_unserializes_stateless(zend_class_entry *ce) return module && (!strcmp(module, "dom") || !strcmp(module, "xsl") || !strcmp(module, "mysqli") || !strcmp(module, "soap")); } +/* Whether the class is an internal one that keeps its state out of its + * properties and declares no serialization API, like Redis, other than the + * ones above, or a user class that extends one without declaring such an API. + * All three functions reject these: unserialize() creates them without that + * state, and some crash when used, or even destroyed, that way. + * deepclone_to_array() and deepclone_hydrate() probe the final ones instead. */ +static zend_never_inline bool dc_needs_internal_state_slow(zend_class_entry *ce) +{ + if (ce->serialize != NULL || ce->__serialize || ce->__unserialize + || ce == php_ce_incomplete_class + || zend_hash_find_known_hash(&ce->function_table, ZSTR_KNOWN(ZEND_STR_SLEEP)) + || zend_hash_find_known_hash(&ce->function_table, ZSTR_KNOWN(ZEND_STR_WAKEUP))) { + return false; + } + + /* User classes inherit create_object from their closest internal parent */ + while (ce && ce->type != ZEND_INTERNAL_CLASS) { + ce = ce->parent; + } + + return ce && ce->create_object != NULL + && !(ce->ce_flags & ZEND_ACC_FINAL) + && !dc_unserializes_stateless(ce); +} + +static zend_always_inline bool dc_needs_internal_state(zend_class_entry *ce) +{ + return UNEXPECTED(ce->create_object != NULL) && dc_needs_internal_state_slow(ce); +} + /* ── Helpers ────────────────────────────────────────────────── */ @@ -590,7 +620,8 @@ static zend_always_inline bool dc_refuses_serialization(zend_class_entry *ce) /* Look up the class of payload objects, rejecting the ones unserialize() can't * create: abstract classes, interfaces, traits and enums, and the classes that - * refuse serialization. */ + * refuse serialization; and the ones it would create without their internal + * state, which deepclone_to_array() rejects too. */ static zend_class_entry *dc_lookup_payload_class(zend_string *class_name) { zend_class_entry *ce = zend_lookup_class(class_name); @@ -598,7 +629,7 @@ static zend_class_entry *dc_lookup_payload_class(zend_string *class_name) if (UNEXPECTED(!ce)) { zend_throw_exception_ex(dc_ce_class_not_found_exception, 0, "Class \"%s\" not found.", ZSTR_VAL(class_name)); - } else if (UNEXPECTED((ce->ce_flags & ZEND_ACC_UNINSTANTIABLE) || dc_refuses_serialization(ce))) { + } else if (UNEXPECTED((ce->ce_flags & ZEND_ACC_UNINSTANTIABLE) || dc_refuses_serialization(ce) || dc_needs_internal_state(ce))) { zend_throw_exception_ex(dc_ce_not_instantiable_exception, 0, "Type \"%s\" is not instantiable.", ZSTR_VAL(ce->name)); ce = NULL; @@ -666,12 +697,7 @@ static uint8_t dc_get_class_info(dc_ctx *ctx, zend_class_entry *ce) } else { zval_ptr_dtor(&probe); } - } else if (ce->type == ZEND_INTERNAL_CLASS - && ce->create_object != NULL - && ce->serialize == NULL - && !(flags & (DC_CI_HAS_SERIALIZE | DC_CI_HAS_UNSERIALIZE | DC_CI_HAS_SLEEP | DC_CI_HAS_WAKEUP)) - && ce != php_ce_incomplete_class - && !dc_unserializes_stateless(ce)) { + } else if (dc_needs_internal_state(ce)) { flags |= DC_CI_NOT_INSTANTIABLE; } @@ -5832,9 +5858,8 @@ PHP_FUNCTION(deepclone_hydrate) } /* Reject classes that cannot function without their constructor, * using the same rules as dc_get_class_info / deepclone_from_array. - * Internal classes are checked and cached; user classes pass unless - * they refuse serialization, checked each time as the cache is - * persistent and their names aren't. */ + * Internal classes are checked and cached; user classes are checked + * each time as the cache is persistent and their names aren't. */ if (UNEXPECTED(ce->type == ZEND_INTERNAL_CLASS)) { /* Per-thread cache (via module globals). Packs ce pointer + ok-bit * into the stored value: low bit is ok, high bits are the ce. A ce @@ -5865,18 +5890,17 @@ PHP_FUNCTION(deepclone_hydrate) if (ok && dc_refuses_serialization(ce)) { ok = false; } - if (ok && ce->create_object != NULL && ce != php_ce_incomplete_class && !has_ser_api + if (ok && dc_needs_internal_state(ce)) { + ok = false; + } else if (ok && ce->create_object != NULL && (ce->ce_flags & ZEND_ACC_FINAL) + && ce != php_ce_incomplete_class && !has_ser_api && !(ce->__serialize) && !(zend_hash_find_known_hash(&ce->function_table, ZSTR_KNOWN(ZEND_STR_SLEEP)))) { - if (ce->ce_flags & ZEND_ACC_FINAL) { - zval probe; - if (object_init_ex(&probe, ce) != SUCCESS || EG(exception)) { - zend_clear_exception(); - ok = false; - } else { - zval_ptr_dtor(&probe); - } - } else if (!dc_unserializes_stateless(ce)) { + zval probe; + if (object_init_ex(&probe, ce) != SUCCESS || EG(exception)) { + zend_clear_exception(); ok = false; + } else { + zval_ptr_dtor(&probe); } } /* Internal final classes with create_object: the engine refuses @@ -5935,7 +5959,7 @@ PHP_FUNCTION(deepclone_hydrate) RETURN_THROWS(); } } - } else if (UNEXPECTED(dc_refuses_serialization(ce))) { + } else if (UNEXPECTED(dc_refuses_serialization(ce) || dc_needs_internal_state(ce))) { zend_throw_exception_ex(dc_ce_not_instantiable_exception, 0, "Type \"%s\" is not instantiable.", ZSTR_VAL(ce->name)); RETURN_THROWS(); diff --git a/tests/deepclone_from_array_internal_state.phpt b/tests/deepclone_from_array_internal_state.phpt new file mode 100644 index 0000000..66bfe55 --- /dev/null +++ b/tests/deepclone_from_array_internal_state.phpt @@ -0,0 +1,83 @@ +--TEST-- +deepclone_from_array() rejects the internal classes that keep their state out of their properties, like deepclone_to_array() and deepclone_hydrate() +--EXTENSIONS-- +deepclone +--FILE-- +getMessage(), "\n"; + } +} + +function payload(string $class): array +{ + // What the polyfill produces for an object with no properties + return ['classes' => $class, 'objectMeta' => 1, 'prepared' => 0]; +} + +check('to_array IteratorIterator', fn () => deepclone_to_array(new IteratorIterator(new ArrayIterator([1])))); +check('hydrate IteratorIterator', fn () => deepclone_hydrate('IteratorIterator')); +check('from_array IteratorIterator', fn () => deepclone_from_array(payload('IteratorIterator'))); +check('from_array iteratoriterator', fn () => deepclone_from_array(payload('iteratoriterator'))); +check('from_array AppendIterator', fn () => deepclone_from_array(payload('AppendIterator'))); + +// Objects holding closures are created as lazy ghosts on PHP 8.4+ +$payload = payload('IteratorIterator') + ['properties' => ['stdClass' => ['f' => [[null, 'strlen']]]], 'resolve' => ['stdClass' => ['f' => [0]]]]; +check('from_array lazy IteratorIterator', fn () => deepclone_from_array($payload, null, true)); + +// Classes of other extensions, when loaded: some crash when used, or even +// destroyed, without their constructor +foreach (['XMLWriter', 'XMLReader', 'ZipArchive', 'Redis', 'RedisCluster', 'Imagick', 'ImagickPixel', 'Relay\Table', 'AMQPConnection', 'APCUIterator'] as $class) { + if (!class_exists($class) || method_exists($class, '__unserialize')) { + continue; + } + $subclass = 'User'.strtr($class, '\\', '_'); + eval("class $subclass extends $class {}"); + foreach ([$class, $subclass] as $class) { + try { + deepclone_from_array(payload($class)); + echo "from_array $class: accepted\n"; + } catch (DeepClone\NotInstantiableException $e) { + if ($e->getMessage() !== "Type \"$class\" is not instantiable.") { + echo "from_array $class: ", $e->getMessage(), "\n"; + } + } + } +} + +// User subclasses, unless they declare a serialization API, and the internal +// classes unserialize() creates all the same +check('to_array UserIterator', fn () => deepclone_to_array(new UserIterator(new ArrayIterator([1])))); +check('from_array UserIterator', fn () => deepclone_from_array(payload('UserIterator'))); +check('hydrate UserIterator', fn () => deepclone_hydrate('UserIterator')); +check('from_array UserIteratorWithWakeup', fn () => deepclone_from_array(payload('UserIteratorWithWakeup'))); +check('hydrate UserIteratorWithWakeup', fn () => deepclone_hydrate('UserIteratorWithWakeup')); +check('from_array MultipleIterator', fn () => deepclone_from_array(payload('MultipleIterator'))); +check('round-trip SplMinHeap', fn () => deepclone_from_array(deepclone_to_array(new SplMinHeap()))); +if (class_exists(DOMNodeList::class) && !deepclone_from_array(payload('DOMNodeList')) instanceof DOMNodeList) { + echo "from_array DOMNodeList: rejected\n"; +} +?> +--EXPECT-- +to_array IteratorIterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. +hydrate IteratorIterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. +from_array IteratorIterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. +from_array iteratoriterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. +from_array AppendIterator: DeepClone\NotInstantiableException: Type "AppendIterator" is not instantiable. +from_array lazy IteratorIterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. +to_array UserIterator: DeepClone\NotInstantiableException: Type "UserIterator" is not instantiable. +from_array UserIterator: DeepClone\NotInstantiableException: Type "UserIterator" is not instantiable. +hydrate UserIterator: DeepClone\NotInstantiableException: Type "UserIterator" is not instantiable. +from_array UserIteratorWithWakeup: UserIteratorWithWakeup +hydrate UserIteratorWithWakeup: UserIteratorWithWakeup +from_array MultipleIterator: MultipleIterator +round-trip SplMinHeap: SplMinHeap diff --git a/tests/deepclone_uninstantiable.phpt b/tests/deepclone_uninstantiable.phpt index 03099aa..30f0c0c 100644 --- a/tests/deepclone_uninstantiable.phpt +++ b/tests/deepclone_uninstantiable.phpt @@ -31,8 +31,8 @@ foreach (['AbstractThing', 'Thing', 'ThingTrait', 'ThingEnum'] as $class) { $payload = ['classes' => 'AbstractThing', 'objectMeta' => 1, 'prepared' => 0, 'properties' => ['stdClass' => ['f' => [[null, 'strlen']]]], 'resolve' => ['stdClass' => ['f' => [0]]]]; check('from_array lazy AbstractThing', fn () => deepclone_from_array($payload, null, true)); -// unserialize() creates the classes whose state serialize() loses, and so -// does deepclone_from_array(), while deepclone_to_array() rejects them +// unserialize() creates the classes whose state serialize() loses, and their +// user subclasses, while the three functions reject them foreach (['IteratorIterator', 'LimitIterator', 'UserIterator'] as $class) { check("unserialize $class", fn () => unserialize('O:'.strlen($class).':"'.$class.'":0:{}')); check("from_array $class", fn () => deepclone_from_array(['classes' => $class, 'objectMeta' => 1, 'prepared' => 0])); @@ -52,11 +52,11 @@ from_array ThingEnum: DeepClone\NotInstantiableException: Type "ThingEnum" is no hydrate ThingEnum: DeepClone\NotInstantiableException: Type "ThingEnum" is not instantiable. from_array lazy AbstractThing: DeepClone\NotInstantiableException: Type "AbstractThing" is not instantiable. unserialize IteratorIterator: IteratorIterator -from_array IteratorIterator: IteratorIterator +from_array IteratorIterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. unserialize LimitIterator: LimitIterator -from_array LimitIterator: LimitIterator +from_array LimitIterator: DeepClone\NotInstantiableException: Type "LimitIterator" is not instantiable. unserialize UserIterator: UserIterator -from_array UserIterator: UserIterator +from_array UserIterator: DeepClone\NotInstantiableException: Type "UserIterator" is not instantiable. to_array IteratorIterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. hydrate IteratorIterator: DeepClone\NotInstantiableException: Type "IteratorIterator" is not instantiable. -hydrate UserIterator: UserIterator +hydrate UserIterator: DeepClone\NotInstantiableException: Type "UserIterator" is not instantiable.