deepclone_hydrate: reject hydrating shell-less internal final classes (BcMath\Number)

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

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) {

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--
+<?php if (!class_exists('BcMath\Number')) die('skip requires BcMath\Number (PHP 8.4+ with ext/bcmath)'); ?>
+--FILE--
+<?php
+use BcMath\Number;
+
+// ── Round-trip: value, scale, shared identity and nesting are preserved.
+//    BcMath\Number is a final internal class with a custom create_object, so
+//    newInstanceWithoutConstructor() is rejected and an empty O: unserialize is
+//    refused by __unserialize(); the round-trip must replay the full state. ──
+$n = new Number('12.34');
+$o = (object) ['a' => $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

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--
+<?php if (!class_exists('BcMath\Number')) die('skip requires BcMath\Number (PHP 8.4+ with ext/bcmath)'); ?>
+--INI--
+error_reporting=E_ALL & ~E_DEPRECATED
+--FILE--
+<?php
+use BcMath\Number;
+
+// deepclone_hydrate() probes instantiability by running an inner unserialize().
+// When invoked from inside an active inline unserialize() (Serializable::
+// unserialize() here, which does not raise serialize_lock), that probe must
+// stay isolated: it must still reject an uninstantiable class, and must not
+// leak its throwaway object into the outer unserialize() and corrupt it.
+class Holder implements Serializable {
+    public string $result = '';
+    public function serialize(): ?string { return ''; }
+    public function unserialize(string $data): void {
+        try {
+            deepclone_hydrate(Number::class);
+            $this->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)