From ce24d38c3750dde7cd95bf4b91d17edb050561d7 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Mon, 8 Jun 2026 20:56:49 +0200 Subject: [PATCH] deepclone_hydrate: reject hydrating shell-less internal final classes BcMath\Number (and any final internal class with a serialization API whose empty serialization payload does not reconstruct an instance) was hydrated by injecting properties into a bare create_object() shell. The engine refuses ReflectionClass::newInstanceWithoutConstructor() for such classes precisely because the shell is not usable on its own; for BcMath\Number the bc_num stays NULL, so the result crashed on first use. deepclone_hydrate() now probes an empty serialization payload (under serialize_lock, so it stays isolated when called from inside another unserialize() such as Serializable::unserialize()) and throws the documented \DeepClone\NotInstantiableException when no valid shell can be built, matching the polyfill. Round-trip via deepclone_to_array() / deepclone_from_array() is unaffected: it replays the real state through __unserialize(). --- CHANGELOG.md | 14 ++++ deepclone.c | 48 +++++++++++++ tests/deepclone_bcmath_number.phpt | 68 +++++++++++++++++++ tests/deepclone_hydrate_probe_reentrancy.phpt | 45 ++++++++++++ 4 files changed, 175 insertions(+) create mode 100644 tests/deepclone_bcmath_number.phpt create mode 100644 tests/deepclone_hydrate_probe_reentrancy.phpt diff --git a/CHANGELOG.md b/CHANGELOG.md index c4da062..a36bdf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `deepclone_hydrate()` now refuses an internal `final` class with a + serialization API whose empty serialization payload cannot reconstruct an + instance (e.g. `BcMath\Number`, whose `bc_num` stays `NULL` until + `__construct()`/`__unserialize()` runs), throwing the documented + `\DeepClone\NotInstantiableException` instead of injecting properties into a + half-built `create_object` shell, which produced an uninitialized object + that crashed on first use. The engine already refuses + `ReflectionClass::newInstanceWithoutConstructor()` for such classes; this + matches that behaviour and the polyfill. The instantiability probe runs under + `serialize_lock` so it stays isolated when `deepclone_hydrate()` is called + from inside another `unserialize()` (e.g. `Serializable::unserialize()`). + Round-trip via `deepclone_to_array()` / `deepclone_from_array()` is + unaffected; it replays the real state through `__unserialize()` + (symfony/symfony#64323). - `deepclone_from_array()` now rejects a malformed payload whose serialized class-name blob (a string whose second byte is `:`) decodes to a non-object via `unserialize()`, instead of storing the scalar/array result and later diff --git a/deepclone.c b/deepclone.c index d6ca65d..18210ec 100644 --- a/deepclone.c +++ b/deepclone.c @@ -3150,6 +3150,54 @@ PHP_FUNCTION(deepclone_hydrate) ok = false; } } + /* Internal final classes with create_object: the engine refuses + * newInstanceWithoutConstructor() for them (INTERNAL + create_object + * + FINAL). Hydrate injects properties into a create_object shell, + * but a class whose validity depends on __construct()/__unserialize() + * (e.g. BcMath\Number, whose bc_num stays NULL until then) cannot be + * built that way. Mirror the polyfill: probe an empty serialization + * payload and refuse hydrate if it does not reconstruct an object. + * Round-trip (deepclone_from_array) is unaffected — it replays the + * real state through __unserialize(). */ + if (ok && has_ser_api && ce->create_object != NULL + && (ce->ce_flags & ZEND_ACC_FINAL) && ce != php_ce_incomplete_class) { + smart_str payload = {0}; + smart_str_appendc(&payload, (ce->serialize != NULL && !has_unser) ? 'C' : 'O'); + smart_str_appendc(&payload, ':'); + smart_str_append_long(&payload, (zend_long) ZSTR_LEN(ce->name)); + smart_str_appendl(&payload, ":\"", 2); + smart_str_append(&payload, ce->name); + smart_str_appendl(&payload, "\":0:{}", 6); + smart_str_0(&payload); + + zval probe; + ZVAL_UNDEF(&probe); + const unsigned char *p = (const unsigned char *) ZSTR_VAL(payload.s); + const unsigned char *pend = p + ZSTR_LEN(payload.s); + /* Raise serialize_lock so the probe gets an isolated + * php_unserialize_data: otherwise, when hydrate runs inside + * an active unserialize (e.g. Serializable::unserialize()), + * PHP_VAR_UNSERIALIZE_INIT would reuse the outer context's + * data and DESTROY would skip var_destroy (level>1), so the + * deferred __unserialize([])/__wakeup() never fires here and + * the probe object leaks into the outer dtor list. */ + BG(serialize_lock)++; + php_unserialize_data_t var_hash; + PHP_VAR_UNSERIALIZE_INIT(var_hash); + bool shell_ok = php_var_unserialize(&probe, &p, pend, &var_hash) + && Z_TYPE(probe) == IS_OBJECT; + PHP_VAR_UNSERIALIZE_DESTROY(var_hash); + BG(serialize_lock)--; + if (EG(exception)) { + zend_clear_exception(); + shell_ok = false; + } + zval_ptr_dtor(&probe); + smart_str_free(&payload); + if (!shell_ok) { + ok = false; + } + } packed = (uintptr_t) ce | (ok ? 1 : 0); zend_hash_update_ptr(hydrate_cache, ce->name, (void *) packed); if (!ok) { diff --git a/tests/deepclone_bcmath_number.phpt b/tests/deepclone_bcmath_number.phpt new file mode 100644 index 0000000..0ff632e --- /dev/null +++ b/tests/deepclone_bcmath_number.phpt @@ -0,0 +1,68 @@ +--TEST-- +deepclone round-trips BcMath\Number, refuses to hydrate it, and never crashes on an uninitialized one (symfony/symfony#64323) +--EXTENSIONS-- +deepclone +--SKIPIF-- + +--FILE-- + $n, 'b' => $n, 'list' => [$n, new Number('5')]]; +$c = deepclone_from_array(deepclone_to_array($o)); + +var_dump($c->a instanceof Number); +var_dump((string) $c->a); +var_dump($c->a->scale); +var_dump($c->a !== $n); // a real clone, not the original +var_dump($c->a === $c->b); // shared identity preserved +var_dump($c->a === $c->list[0]); // preserved across the whole graph +var_dump((string) $c->list[1]); + +// ── A top-level Number round-trips and stays usable. ── +$top = deepclone_from_array(deepclone_to_array(new Number('99.999'))); +var_dump((string) $top, $top->scale, (string) $top->add('0.5')); + +// ── deepclone_hydrate() injects properties into an empty shell, which cannot +// produce a valid BcMath\Number; it must refuse rather than yield a broken +// instance. ── +try { + deepclone_hydrate(Number::class, ['value' => '7.5']); +} catch (\DeepClone\NotInstantiableException $e) { + echo $e->getMessage(), "\n"; +} + +// ── Defensive: an uninitialized instance (forced here via a hand-mutated +// payload that drops the __unserialize replay) must throw a clean Error on +// use, never segfault. ── +$d = deepclone_to_array(new Number('1')); +$d['objectMeta'][0][1] = 0; // clear the __unserialize flag +$d['states'] = []; // and drop the replay +$u = deepclone_from_array($d); +var_dump($u instanceof Number); +foreach (['toString' => fn() => (string) $u, 'clone' => fn() => clone $u, 'add' => fn() => $u->add(1)] as $op => $f) { + try { $f(); echo "$op: no error\n"; } + catch (\Error $e) { echo "$op: ", $e->getMessage(), "\n"; } +} +?> +--EXPECT-- +bool(true) +string(5) "12.34" +int(2) +bool(true) +bool(true) +bool(true) +string(1) "5" +string(6) "99.999" +int(3) +string(7) "100.499" +Class "BcMath\Number" is not instantiable. +bool(true) +toString: The BcMath\Number object has not been correctly initialized by its constructor +clone: The BcMath\Number object has not been correctly initialized by its constructor +add: The BcMath\Number object has not been correctly initialized by its constructor diff --git a/tests/deepclone_hydrate_probe_reentrancy.phpt b/tests/deepclone_hydrate_probe_reentrancy.phpt new file mode 100644 index 0000000..ffa3f36 --- /dev/null +++ b/tests/deepclone_hydrate_probe_reentrancy.phpt @@ -0,0 +1,45 @@ +--TEST-- +deepclone_hydrate() instantiability probe stays isolated when called inside another unserialize() (symfony/symfony#64323) +--EXTENSIONS-- +deepclone +--SKIPIF-- + +--INI-- +error_reporting=E_ALL & ~E_DEPRECATED +--FILE-- +result = 'allowed'; + } catch (\DeepClone\NotInstantiableException $e) { + $this->result = 'refused'; + } + } +} + +$h = unserialize('C:6:"Holder":0:{}'); +var_dump($h instanceof Holder); // the outer unserialize() succeeded +var_dump($h->result); // the probe still rejected the class + +// The outer context is not corrupted: a real BcMath\Number unserialize() that +// runs afterwards (its deferred __unserialize fires in the same var_destroy) +// works cleanly. +$n = unserialize(serialize(new Number('3.5'))); +var_dump((string) $n, $n->scale); +?> +--EXPECT-- +bool(true) +string(7) "refused" +string(3) "3.5" +int(1)