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