From 0fb03e3c59236aa31470bb25b12dff35d61c2346 Mon Sep 17 00:00:00 2001 From: Alb3e3 <74142887+Alb3e3@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:56:21 +0400 Subject: [PATCH 1/2] Fix use-after-free serializing an array grown by an element's hook The IS_ARRAY case of php_var_serialize_intern() walked the array's HashTable without holding a reference across php_var_serialize_nested_data(), which recurses into user hooks (__serialize, __sleep, Serializable::serialize). A hook that grows the same array through a by-reference alias reallocs the backing store mid-walk, so the iterator reads freed memory. Hold a ref across the walk, as the object path and var_dump/var_export already do, so the append separates a copy instead of reallocating in place. This is a backport of cc8abaf9f (GH-22714), which landed on master only. GH-23385 needs it on this branch as well. --- .../serialize_array_ref_reentrancy.phpt | 21 +++++++++++++++++++ ext/standard/var.c | 8 +++++-- 2 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/serialize/serialize_array_ref_reentrancy.phpt diff --git a/ext/standard/tests/serialize/serialize_array_ref_reentrancy.phpt b/ext/standard/tests/serialize/serialize_array_ref_reentrancy.phpt new file mode 100644 index 000000000000..490fa4f6c3b0 --- /dev/null +++ b/ext/standard/tests/serialize/serialize_array_ref_reentrancy.phpt @@ -0,0 +1,21 @@ +--TEST-- +serialize(): a by-reference __serialize() that grows the array being walked must not free it +--FILE-- +ref[] = 'x' . $i; + } + return ['d' => 1]; + } +} +$g = new G(); +$inner = [$g, 'tail']; +$g->ref = &$inner; +$top = [&$inner]; +var_dump(serialize($top)); +?> +--EXPECT-- +string(59) "a:1:{i:0;a:2:{i:0;O:1:"G":1:{s:1:"d";i:1;}i:1;s:4:"tail";}}" diff --git a/ext/standard/var.c b/ext/standard/var.c index 99f28366ffdb..d98b40bdd48c 100644 --- a/ext/standard/var.c +++ b/ext/standard/var.c @@ -1319,13 +1319,17 @@ static void php_var_serialize_intern(smart_str *buf, zval *struc, php_serialize_ zend_release_properties(myht); return; } - case IS_ARRAY: + case IS_ARRAY: { smart_str_appendl(buf, "a:", 2); myht = Z_ARRVAL_P(struc); + bool rcn = !is_root && (in_rcn_array || GC_REFCOUNT(myht) > 1); + GC_TRY_ADDREF(myht); php_var_serialize_nested_data( buf, struc, myht, zend_array_count(myht), /* incomplete_class */ 0, var_hash, - !is_root && (in_rcn_array || GC_REFCOUNT(myht) > 1)); + rcn); + GC_TRY_DTOR_NO_REF(myht); return; + } case IS_REFERENCE: struc = Z_REFVAL_P(struc); goto again; From 68331028984cd4ae0a1d75dc381565f4d7b4cf9f Mon Sep 17 00:00:00 2001 From: Alb3e3 <74142887+Alb3e3@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:56:21 +0400 Subject: [PATCH 2/2] Fix GH-23385: Use-after-free in SplDoublyLinkedList::serialize() SplDoublyLinkedList::serialize() walks the list and hands each element's zval to php_var_serialize(). Serializing an element can call back into userland (__serialize, __sleep, Serializable::serialize), and that code can remove the very element being serialized from the list. The loop already takes a reference on the *next* element for exactly this reason, but not on the current one. Once offsetUnset() drops the last reference to it the element is freed, while php_var_serialize() still holds a pointer into it as its `struc` argument and dereferences it for every value it walks. Take a reference on the current element as well, so it survives the call. The element is already designed to outlive its removal from the list, see the "Keep consistency if element is kept alive" path in offsetUnset(). --- ext/spl/spl_dllist.c | 5 ++++ ext/spl/tests/gh23385.phpt | 57 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 ext/spl/tests/gh23385.phpt diff --git a/ext/spl/spl_dllist.c b/ext/spl/spl_dllist.c index 2ac7980a86cb..5399b685b5cf 100644 --- a/ext/spl/spl_dllist.c +++ b/ext/spl/spl_dllist.c @@ -1019,10 +1019,15 @@ PHP_METHOD(SplDoublyLinkedList, serialize) smart_str_appendc(&buf, ':'); next = current->next; + /* Serializing an element can run user code, which may remove this + * element (and its neighbour) from the list, so hold a reference on + * both for the duration of the call. */ + SPL_LLIST_ADDREF(current); SPL_LLIST_CHECK_ADDREF(next); php_var_serialize(&buf, ¤t->data, &var_hash); + SPL_LLIST_DELREF(current); SPL_LLIST_CHECK_DELREF_EX(next, break;); current = next; diff --git a/ext/spl/tests/gh23385.phpt b/ext/spl/tests/gh23385.phpt new file mode 100644 index 000000000000..236f0cbdf817 --- /dev/null +++ b/ext/spl/tests/gh23385.phpt @@ -0,0 +1,57 @@ +--TEST-- +GH-23385 (Use-after-free in SplDoublyLinkedList::serialize()) +--FILE-- +push([new RemoveSelf(), [1, 2, 3]]); +$list->push("tail"); +var_dump($list->serialize()); +var_dump($list->count()); + +class RemoveNext { + public function __serialize(): array { + global $list2; + unset($list2[1]); + return []; + } +} + +$list2 = new SplDoublyLinkedList(); +$list2->push(new RemoveNext()); +$list2->push("removed"); +$list2->push("after"); +var_dump($list2->serialize()); +var_dump($list2->count()); + +class RemoveAll { + public function __serialize(): array { + global $list3; + while (!$list3->isEmpty()) { + $list3->pop(); + } + return []; + } +} + +$list3 = new SplDoublyLinkedList(); +$list3->push([new RemoveAll(), [1, 2]]); +$list3->push("x"); +$list3->push("y"); +var_dump($list3->serialize()); +var_dump($list3->count()); +?> +--EXPECTF-- +string(%d) "i:0;:a:2:{i:0;O:10:"RemoveSelf":0:{}i:1;a:3:{i:0;i:1;i:1;i:2;i:2;i:3;}}:s:4:"tail";" +int(1) +string(%d) "i:0;:O:10:"RemoveNext":0:{}" +int(2) +string(%d) "i:0;:a:2:{i:0;O:9:"RemoveAll":0:{}i:1;a:2:{i:0;i:1;i:1;i:2;}}" +int(0)