Skip to content

Fixes infinite recursion introduced by patch to SplFixedArray #8105

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.

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

Merged
merged 1 commit into from
Feb 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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_fixedarray.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ ZEND_GET_MODULE(spl_fixedarray)
typedef struct _spl_fixedarray {
zend_long size;
zval *elements;
/* True if this was modified after the last call to get_properties or the hash table wasn't rebuilt. */
bool should_rebuild_properties;
} spl_fixedarray;

typedef struct _spl_fixedarray_object {
Expand Down Expand Up @@ -104,6 +106,7 @@ static void spl_fixedarray_init(spl_fixedarray *array, zend_long size)
array->size = 0; /* reset size in case ecalloc() fails */
array->elements = safe_emalloc(size, sizeof(zval), 0);
array->size = size;
array->should_rebuild_properties = true;
spl_fixedarray_init_elems(array, 0, size);
} else {
spl_fixedarray_default_ctor(array);
Expand Down Expand Up @@ -166,6 +169,7 @@ static void spl_fixedarray_resize(spl_fixedarray *array, zend_long size)
/* nothing to do */
return;
}
array->should_rebuild_properties = true;

/* first initialization */
if (array->size == 0) {
Expand Down Expand Up @@ -205,6 +209,22 @@ static HashTable* spl_fixedarray_object_get_properties(zend_object *obj)
HashTable *ht = zend_std_get_properties(obj);

if (!spl_fixedarray_empty(&intern->array)) {
/*
* Usually, the reference count of the hash table is 1,
* except during cyclic reference cycles.
*
* Maintain the DEBUG invariant that a hash table isn't modified during iteration,
* and avoid unnecessary work rebuilding a hash table for unmodified properties.
*
* See https://github.com/php/php-src/issues/8079 and ext/spl/tests/fixedarray_022.phpt
* Also see https://github.com/php/php-src/issues/8044 for alternate considered approaches.
*/
if (!intern->array.should_rebuild_properties) {
/* Return the same hash table so that recursion cycle detection works in internal functions. */
return ht;
}
intern->array.should_rebuild_properties = false;

zend_long j = zend_hash_num_elements(ht);

if (GC_REFCOUNT(ht) > 1) {
Expand Down Expand Up @@ -354,6 +374,9 @@ static zval *spl_fixedarray_object_read_dimension(zend_object *object, zval *off
}
return &EG(uninitialized_zval);
}
if (type != BP_VAR_IS && type != BP_VAR_R) {
intern->array.should_rebuild_properties = true;
}

return spl_fixedarray_object_read_dimension_helper(intern, offset);
}
Expand All @@ -378,6 +401,7 @@ static void spl_fixedarray_object_write_dimension_helper(spl_fixedarray_object *
zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
return;
} else {
intern->array.should_rebuild_properties = true;
/* Fix #81429 */
zval *ptr = &(intern->array.elements[index]);
zval tmp;
Expand Down Expand Up @@ -425,6 +449,7 @@ static void spl_fixedarray_object_unset_dimension_helper(spl_fixedarray_object *
zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
return;
} else {
intern->array.should_rebuild_properties = true;
zval_ptr_dtor(&(intern->array.elements[index]));
ZVAL_NULL(&intern->array.elements[index]);
}
Expand Down
36 changes: 36 additions & 0 deletions ext/spl/tests/fixedarray_023.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
--TEST--
SPL: FixedArray: Infinite loop in var_export bugfix
--FILE--
<?php
call_user_func(function () {
$x = new SplFixedArray(4);
$x[0] = NAN; // Test NAN just in case this check is incorrectly refactored to use zend_is_identical
$x[1] = 0.0;
$x[2] = $x;
$x[3] = $x;
var_export($x);
echo "\n";
$x[1] = -0.0;
debug_zval_dump($x);
});
?>
--EXPECTF--
Warning: var_export does not handle circular references in %s on line 8

Warning: var_export does not handle circular references in %s on line 8
SplFixedArray::__set_state(array(
0 => NAN,
1 => 0.0,
2 => NULL,
3 => NULL,
))
object(SplFixedArray)#2 (4) refcount(6){
[0]=>
float(NAN)
[1]=>
float(-0)
[2]=>
*RECURSION*
[3]=>
*RECURSION*
}