Skip to content

Fix use-after-free when json_encode() re-enters an ArrayObject #22921

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iliaal wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
+64 −6
Open

Fix use-after-free when json_encode() re-enters an ArrayObject #22921

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions ext/spl/spl_array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
}
Expand Down
51 changes: 51 additions & 0 deletions ext/spl/tests/ArrayObject_modified_during_json_encode.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
--TEST--
Modifying an ArrayObject or ArrayIterator from within json_encode()
--FILE--
<?php

class Mutator implements JsonSerializable
{
public function __construct(private object $target) {}

public function jsonSerialize(): mixed
{
$this->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
Loading