Skip to content

Commit 0bc6dc3

Browse files
deepclone_to_array: drop unsound refcount==1 pool-skip optimization
dc_copy_value had a fast path that skipped the object-pool lookup when Z_REFCOUNT_P(src) == 1 and no __serialize, on the assumption that refcount==1 objects are only visited once. That assumption breaks when the object is reached via a SHARED parent array: the parent's refcount is > 1, but the contained object's is 1, and the parent is walked multiple times, so the object is visited twice. On the second visit, the skip bypassed the pool lookup, fell through to dc_process_object, and tripped zend_hash_index_add_new's assertion (slot already occupied from the first visit). Fix: always do the pool lookup. The save-one-hash-find per object is not worth the soundness risk. Found by libFuzzer (roundtrip harness + duplicated shared containers).
1 parent 3e0bf4f commit 0bc6dc3

1 file changed

Lines changed: 5 additions & 4 deletions

File tree

deepclone.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,10 +1050,11 @@ static void dc_copy_value(dc_ctx *ctx, zval *src, zval *dst, zval *mask_dst)
10501050
ctx->is_static = 0;
10511051
{
10521052
uint32_t handle = Z_OBJ_HANDLE_P(src);
1053-
/* Skip pool lookup for refcount==1 objects without __serialize */
1054-
zval *pooled = (Z_REFCOUNT_P(src) == 1 && Z_OBJCE_P(src)->__serialize == NULL)
1055-
? NULL
1056-
: zend_hash_index_find(&ctx->object_pool, handle);
1053+
/* Always do the pool lookup — an object with refcount==1 can still be
1054+
* reached twice if a SHARED parent array is walked from multiple paths.
1055+
* The earlier "skip on refcount==1" optimization tripped add_new() on
1056+
* the second visit, since pool state persists across walks. */
1057+
zval *pooled = zend_hash_index_find(&ctx->object_pool, handle);
10571058
if (UNEXPECTED(pooled != NULL)) {
10581059
ctx->objects_count++;
10591060
dc_pool_entry *entry = (dc_pool_entry *)Z_PTR_P(pooled);

0 commit comments

Comments
 (0)