Skip to content

Commit b4cb516

Browse files
deepclone_to_array: resolve INDIRECT slots in retained __serialize() states
Before PHP 8.3, Random\Randomizer::__serialize() returns the object's raw property table with only the table itself addref'd; its "engine" entry is an IS_INDIRECT slot pointing into the object (fixed for 8.3 by php-src commit c5fa7696e64, never backported to the by-then security-only 8.2). deepclone_to_array() walks such a state with dc_copy_array(), which passed IS_INDIRECT slots to dc_copy_value() unresolved, so the payload retained pointers into the source object. Once a Randomizer that did not outlive its payload was released, deepclone_from_array() dereferenced freed memory and crashed. Resolve indirects (and skip resolved UNDEF slots) at the start of the walk, exactly like the native serializer and the existing build_scoped_props loop already do.
1 parent e4c510b commit b4cb516

2 files changed

Lines changed: 45 additions & 0 deletions

File tree

deepclone.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -964,6 +964,17 @@ static void dc_copy_array(dc_ctx *ctx, HashTable *src_ht, zval *dst, zval *mask_
964964
zend_hash_real_init_mixed(Z_ARRVAL_P(mask_dst));
965965

966966
ZEND_HASH_FOREACH_KEY_VAL(src_ht, idx, key, src_val) {
967+
/* __serialize() may return the object's raw property table (e.g.
968+
* Random\Randomizer before PHP 8.3), where declared properties are
969+
* IS_INDIRECT slots into the object. Resolve them like the native
970+
* serializer does, or the payload would retain pointers that dangle
971+
* once the source object is released. */
972+
if (UNEXPECTED(Z_TYPE_P(src_val) == IS_INDIRECT)) {
973+
src_val = Z_INDIRECT_P(src_val);
974+
if (Z_TYPE_P(src_val) == IS_UNDEF) {
975+
continue;
976+
}
977+
}
967978
zval undef, null_marker;
968979
ZVAL_UNDEF(&undef);
969980
ZVAL_NULL(&null_marker);

tests/deepclone_randomizer.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
deepclone round-trips a Random\Randomizer that does not outlive its payload
3+
--EXTENSIONS--
4+
deepclone
5+
--SKIPIF--
6+
<?php if (PHP_VERSION_ID < 80200) die('skip requires PHP 8.2'); ?>
7+
--FILE--
8+
<?php
9+
10+
// Before PHP 8.3, Randomizer::__serialize() returns its raw property table,
11+
// whose "engine" slot is an IS_INDIRECT pointer into the object. The payload
12+
// must not retain it once the source Randomizer is released.
13+
$expected = (new Random\Randomizer(new Random\Engine\Mt19937(42)))->getInt(1, PHP_INT_MAX);
14+
15+
$d = deepclone_to_array(new Random\Randomizer(new Random\Engine\Mt19937(42)));
16+
gc_collect_cycles();
17+
$clone = deepclone_from_array($d);
18+
var_dump($clone instanceof Random\Randomizer);
19+
var_dump($expected === $clone->getInt(1, PHP_INT_MAX));
20+
21+
// Same with the Randomizer nested in a temporary object graph
22+
$g = deepclone_from_array(deepclone_to_array((object) ['list' => [(object) ['r' => new Random\Randomizer(new Random\Engine\Mt19937(9))]]]));
23+
var_dump($g->list[0]->r instanceof Random\Randomizer);
24+
25+
// And behind a shared identity
26+
$r = new Random\Randomizer(new Random\Engine\Mt19937(5));
27+
$c = deepclone_from_array(deepclone_to_array([$r, $r]));
28+
var_dump($c[0] === $c[1]);
29+
?>
30+
--EXPECT--
31+
bool(true)
32+
bool(true)
33+
bool(true)
34+
bool(true)

0 commit comments

Comments
 (0)