Skip to content

Fix GH-15833: Segmentation fault (access null pointer) in ext/spl/spl_array.c #17235

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

Closed
wants to merge 4 commits into from
Closed

Fix GH-15833: Segmentation fault (access null pointer) in ext/spl/spl_array.c #17235

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
34f1450
Fix GH-15833: Segmentation fault (access null pointer) in ext/spl/spl…
nielsdos Dec 21, 2024
013582b
fix?
nielsdos Dec 22, 2024
7b3a1f1
update test contents too ofc
nielsdos Dec 22, 2024
456c632
review
nielsdos Dec 25, 2024
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
25 changes: 25 additions & 0 deletions ext/spl/spl_array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ PHPAPI zend_class_entry *spl_ce_ArrayObject;

typedef struct _spl_array_object {
zval array;
HashTable *sentinel_array;
uint32_t ht_iter;
int ar_flags;
unsigned char nApplyCount;
Expand Down Expand Up @@ -74,6 +75,19 @@ static inline HashTable **spl_array_get_hash_table_ptr(spl_array_object* intern)
return &Z_ARRVAL(intern->array);
} else {
zend_object *obj = Z_OBJ(intern->array);
/* Since we're directly playing with the properties table, we shall initialize the lazy object directly.
* If we don't, it's possible to continue working with the wrong object in case we're using a proxy. */
if (UNEXPECTED(zend_lazy_object_must_init(obj))) {
obj = zend_lazy_object_init(obj);
Comment on lines +78 to +81
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

zend_lazy_object_init() may return NULL when initialization fails (in which case EG(exception) is set, too).

Unfortunately I don't see other solution than to return NULL from spl_array_get_hash_table_ptr and to handle that in every caller. (An alternative would be to return a ptr-ptr to an empty array, and to rely on the exception to discard the result of the operation, but then we need to manage that array somehow.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch, okay, I'll do the NULL thing then :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I believe we can just store the empty array in intern->array. I pushed a commit that does that, so the total patch stays simple. The only downside is that any further action won't do anything on the object, which may be fine as the object failed initialization anyway. What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only downside is that any further action won't do anything on the object

I would prefer if further actions on the ArrayObject was carried normally, causing a new attempt to initialize the object and probably a new exception. Otherwise the silent failure could be surprising.

Storing the empty array in a new field in spl_array_object would be fine I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I also added some checks to avoid unexpected modification to the sentinel

if (UNEXPECTED(!obj)) {
if (!intern->sentinel_array) {
intern->sentinel_array = zend_new_array(0);
}
return &intern->sentinel_array;
}
}
/* should no longer be lazy */
ZEND_ASSERT(!zend_lazy_object_must_init(obj));
/* rebuild properties */
zend_std_get_properties_ex(obj);
if (GC_REFCOUNT(obj->properties) > 1) {
Expand Down Expand Up @@ -129,6 +143,10 @@ static void spl_array_object_free_storage(zend_object *object)
zend_hash_iterator_del(intern->ht_iter);
}

if (UNEXPECTED(intern->sentinel_array)) {
zend_array_release(intern->sentinel_array);
}

zend_object_std_dtor(&intern->std);

zval_ptr_dtor(&intern->array);
Expand Down Expand Up @@ -489,6 +507,9 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
uint32_t refcount = 0;
if (!offset || Z_TYPE_P(offset) == IS_NULL) {
ht = spl_array_get_hash_table(intern);
if (UNEXPECTED(ht == intern->sentinel_array)) {
return;
}
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
zend_hash_next_index_insert(ht, value);

Expand All @@ -505,6 +526,10 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
}

ht = spl_array_get_hash_table(intern);
if (UNEXPECTED(ht == intern->sentinel_array)) {
spl_hash_key_release(&key);
return;
}
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
if (key.key) {
zend_hash_update_ind(ht, key.key, value);
Expand Down
22 changes: 22 additions & 0 deletions ext/spl/tests/gh15833_1.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
GH-15833 (Segmentation fault (access null pointer) in ext/spl/spl_array.c)
--CREDITS--
YuanchengJiang
--FILE--
<?php
class C {
public int $a = 1;
}
$reflector = new ReflectionClass(C::class);
$obj = $reflector->newLazyProxy(function ($obj) {
$obj = new C();
return $obj;
});
$recursiveArrayIterator = new RecursiveArrayIterator($obj);
var_dump($recursiveArrayIterator->current());
$recursiveArrayIterator->next();
var_dump($recursiveArrayIterator->current());
?>
--EXPECT--
int(1)
NULL
41 changes: 41 additions & 0 deletions ext/spl/tests/gh15833_2.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
--TEST--
GH-15833 (Segmentation fault (access null pointer) in ext/spl/spl_array.c)
--CREDITS--
YuanchengJiang
--FILE--
<?php
class C {
public int $a = 1;
}
$reflector = new ReflectionClass(C::class);
$obj = $reflector->newLazyProxy(function ($obj) {
static $counter = 0;
throw new Error('nope ' . ($counter++));
});
$recursiveArrayIterator = new RecursiveArrayIterator($obj);
try {
var_dump($recursiveArrayIterator->current());
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
try {
var_dump($recursiveArrayIterator->current());
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
try {
$recursiveArrayIterator->next();
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
try {
var_dump($recursiveArrayIterator->current());
} catch (Error $e) {
echo $e->getMessage(), "\n";
}
?>
--EXPECT--
nope 0
nope 1
nope 2
nope 3
Loading