Skip to content

Commit cd79217

Browse files
committed
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
1 parent 2804d09 commit cd79217

2 files changed

Lines changed: 64 additions & 6 deletions

File tree

ext/spl/spl_array.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,28 @@ static inline spl_array_object *spl_array_from_obj(zend_object *obj) /* {{{ */ {
6060

6161
#define Z_SPLARRAY_P(zv) spl_array_from_obj(Z_OBJ_P((zv)))
6262

63+
static zend_always_inline void spl_array_separate_properties(zend_object *obj)
64+
{
65+
if (GC_REFCOUNT(obj->properties) > 1) {
66+
if (EXPECTED(!(GC_FLAGS(obj->properties) & IS_ARRAY_IMMUTABLE))) {
67+
GC_DELREF(obj->properties);
68+
}
69+
obj->properties = zend_array_dup(obj->properties);
70+
}
71+
}
72+
6373
static inline HashTable **spl_array_get_hash_table_ptr(spl_array_object* intern) { /* {{{ */
6474
//??? TODO: Delay duplication for arrays; only duplicate for write operations
6575
if (intern->ar_flags & SPL_ARRAY_IS_SELF) {
6676
/* rebuild properties */
6777
zend_std_get_properties_ex(&intern->std);
78+
spl_array_separate_properties(&intern->std);
6879
return &intern->std.properties;
6980
} else if (intern->ar_flags & SPL_ARRAY_USE_OTHER) {
7081
spl_array_object *other = Z_SPLARRAY_P(&intern->array);
7182
return spl_array_get_hash_table_ptr(other);
7283
} else if (Z_TYPE(intern->array) == IS_ARRAY) {
84+
SEPARATE_ARRAY(&intern->array);
7385
return &Z_ARRVAL(intern->array);
7486
} else {
7587
zend_object *obj = Z_OBJ(intern->array);
@@ -88,12 +100,7 @@ static inline HashTable **spl_array_get_hash_table_ptr(spl_array_object* intern)
88100
ZEND_ASSERT(!zend_lazy_object_must_init(obj));
89101
/* rebuild properties */
90102
zend_std_get_properties_ex(obj);
91-
if (GC_REFCOUNT(obj->properties) > 1) {
92-
if (EXPECTED(!(GC_FLAGS(obj->properties) & IS_ARRAY_IMMUTABLE))) {
93-
GC_DELREF(obj->properties);
94-
}
95-
obj->properties = zend_array_dup(obj->properties);
96-
}
103+
spl_array_separate_properties(obj);
97104
return &obj->properties;
98105
}
99106
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
--TEST--
2+
Modifying an ArrayObject or ArrayIterator from within json_encode()
3+
--FILE--
4+
<?php
5+
6+
class Mutator implements JsonSerializable
7+
{
8+
public function __construct(private object $target) {}
9+
10+
public function jsonSerialize(): mixed
11+
{
12+
$this->target['b'] = 'mutated';
13+
for ($i = 0; $i < 64; $i++) {
14+
$this->target["appended$i"] = 'appended';
15+
}
16+
$ref = &$this->target['by-reference'];
17+
$ref = 'by-reference';
18+
return 'serialized';
19+
}
20+
}
21+
22+
$self = new ArrayObject();
23+
$self->exchangeArray($self);
24+
25+
$containers = [
26+
'array-backed ArrayObject' => new ArrayObject(),
27+
'array-backed ArrayIterator' => new ArrayIterator(),
28+
'self-backed ArrayObject' => $self,
29+
'object-backed ArrayObject' => new ArrayObject(new stdClass()),
30+
];
31+
32+
foreach ($containers as $label => $container) {
33+
$container['a'] = null;
34+
$container['b'] = 'second';
35+
$container['c'] = 'third';
36+
$container['a'] = new Mutator($container);
37+
38+
echo $label, ': ', json_encode($container), "\n";
39+
echo 'count: ', count($container), "\n";
40+
}
41+
42+
?>
43+
--EXPECT--
44+
array-backed ArrayObject: {"a":"serialized","b":"second","c":"third"}
45+
count: 68
46+
array-backed ArrayIterator: {"a":"serialized","b":"second","c":"third"}
47+
count: 68
48+
self-backed ArrayObject: {"a":"serialized","b":"second","c":"third"}
49+
count: 68
50+
object-backed ArrayObject: {"a":"serialized","b":"second","c":"third"}
51+
count: 68

0 commit comments

Comments
 (0)