From cd79217e60b60ab9e95bc872f5324a69eb280106 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 29 Jul 2026 07:25:12 -0400 Subject: [PATCH] Fix use-after-free when json_encode() re-enters an ArrayObject spl_array_get_hash_table_ptr() hands out a pointer into the ArrayObject storage without checking whether anything else holds a reference to it. json_encode() takes such a reference for the length of the encode and then runs userland through JsonSerializable::jsonSerialize() and property hooks, so an insert from there grows the table and frees the buckets the encoder is iterating. Separate the array-backed and self-backed cases the way the object-backed case in the same function already does. Closes GH-22921 --- ext/spl/spl_array.c | 19 ++++--- ...rayObject_modified_during_json_encode.phpt | 51 +++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 ext/spl/tests/ArrayObject_modified_during_json_encode.phpt diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index b60b3b9c0dac..5fb499444cfa 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -60,16 +60,28 @@ static inline spl_array_object *spl_array_from_obj(zend_object *obj) /* {{{ */ { #define Z_SPLARRAY_P(zv) spl_array_from_obj(Z_OBJ_P((zv))) +static zend_always_inline void spl_array_separate_properties(zend_object *obj) +{ + if (GC_REFCOUNT(obj->properties) > 1) { + if (EXPECTED(!(GC_FLAGS(obj->properties) & IS_ARRAY_IMMUTABLE))) { + GC_DELREF(obj->properties); + } + obj->properties = zend_array_dup(obj->properties); + } +} + static inline HashTable **spl_array_get_hash_table_ptr(spl_array_object* intern) { /* {{{ */ //??? TODO: Delay duplication for arrays; only duplicate for write operations if (intern->ar_flags & SPL_ARRAY_IS_SELF) { /* rebuild properties */ zend_std_get_properties_ex(&intern->std); + spl_array_separate_properties(&intern->std); return &intern->std.properties; } else if (intern->ar_flags & SPL_ARRAY_USE_OTHER) { spl_array_object *other = Z_SPLARRAY_P(&intern->array); return spl_array_get_hash_table_ptr(other); } else if (Z_TYPE(intern->array) == IS_ARRAY) { + SEPARATE_ARRAY(&intern->array); return &Z_ARRVAL(intern->array); } else { zend_object *obj = Z_OBJ(intern->array); @@ -88,12 +100,7 @@ static inline HashTable **spl_array_get_hash_table_ptr(spl_array_object* intern) ZEND_ASSERT(!zend_lazy_object_must_init(obj)); /* rebuild properties */ zend_std_get_properties_ex(obj); - if (GC_REFCOUNT(obj->properties) > 1) { - if (EXPECTED(!(GC_FLAGS(obj->properties) & IS_ARRAY_IMMUTABLE))) { - GC_DELREF(obj->properties); - } - obj->properties = zend_array_dup(obj->properties); - } + spl_array_separate_properties(obj); return &obj->properties; } } diff --git a/ext/spl/tests/ArrayObject_modified_during_json_encode.phpt b/ext/spl/tests/ArrayObject_modified_during_json_encode.phpt new file mode 100644 index 000000000000..5c0f0345a895 --- /dev/null +++ b/ext/spl/tests/ArrayObject_modified_during_json_encode.phpt @@ -0,0 +1,51 @@ +--TEST-- +Modifying an ArrayObject or ArrayIterator from within json_encode() +--FILE-- +target['b'] = 'mutated'; + for ($i = 0; $i < 64; $i++) { + $this->target["appended$i"] = 'appended'; + } + $ref = &$this->target['by-reference']; + $ref = 'by-reference'; + return 'serialized'; + } +} + +$self = new ArrayObject(); +$self->exchangeArray($self); + +$containers = [ + 'array-backed ArrayObject' => new ArrayObject(), + 'array-backed ArrayIterator' => new ArrayIterator(), + 'self-backed ArrayObject' => $self, + 'object-backed ArrayObject' => new ArrayObject(new stdClass()), +]; + +foreach ($containers as $label => $container) { + $container['a'] = null; + $container['b'] = 'second'; + $container['c'] = 'third'; + $container['a'] = new Mutator($container); + + echo $label, ': ', json_encode($container), "\n"; + echo 'count: ', count($container), "\n"; +} + +?> +--EXPECT-- +array-backed ArrayObject: {"a":"serialized","b":"second","c":"third"} +count: 68 +array-backed ArrayIterator: {"a":"serialized","b":"second","c":"third"} +count: 68 +self-backed ArrayObject: {"a":"serialized","b":"second","c":"third"} +count: 68 +object-backed ArrayObject: {"a":"serialized","b":"second","c":"third"} +count: 68