Skip to content

Commit e912c02

Browse files
GirgiasGirgias
committed
Revert 49b2ff5 to fix bug GH-21499
The fix for this was to take hold of a pointer of the bucket, something that should not be done as it causes memory corruptions Closes GH-21532, GH-21520, GH-21499
1 parent b15c597 commit e912c02

File tree

6 files changed

+30
-95
lines changed

6 files changed

+30
-95
lines changed

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@ PHP NEWS
66
. Fixed bug GH-19983 (GC assertion failure with fibers, generators and
77
destructors). (iliaal)
88

9+
- SPL:
10+
. Fixed bug GH-21499 (RecursiveArrayIterator getChildren UAF after parent
11+
free). (Girgias)
12+
913
09 Apr 2026, PHP 8.4.20
1014

1115
- Bz2:

‎ext/spl/spl_array.c‎

Lines changed: 1 addition & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,6 @@ typedef struct _spl_array_object {
4444
uint32_t ht_iter;
4545
int ar_flags;
4646
unsigned char nApplyCount;
47-
bool is_child;
48-
Bucket *bucket;
4947
zend_function *fptr_offset_get;
5048
zend_function *fptr_offset_set;
5149
zend_function *fptr_offset_has;
@@ -166,8 +164,6 @@ static zend_object *spl_array_object_new_ex(zend_class_entry *class_type, zend_o
166164
object_properties_init(&intern->std, class_type);
167165

168166
intern->ar_flags = 0;
169-
intern->is_child = false;
170-
intern->bucket = NULL;
171167
intern->ce_get_iterator = spl_ce_ArrayIterator;
172168
if (orig) {
173169
spl_array_object *other = spl_array_from_obj(orig);
@@ -464,22 +460,6 @@ static zval *spl_array_read_dimension(zend_object *object, zval *offset, int typ
464460
return spl_array_read_dimension_ex(1, object, offset, type, rv);
465461
} /* }}} */
466462

467-
/*
468-
* The assertion(HT_ASSERT_RC1(ht)) failed because the refcount was increased manually when intern->is_child is true.
469-
* We have to set the refcount to 1 to make assertion success and restore the refcount to the original value after
470-
* modifying the array when intern->is_child is true.
471-
*/
472-
static uint32_t spl_array_set_refcount(bool is_child, HashTable *ht, uint32_t refcount) /* {{{ */
473-
{
474-
uint32_t old_refcount = 0;
475-
if (is_child) {
476-
old_refcount = GC_REFCOUNT(ht);
477-
GC_SET_REFCOUNT(ht, refcount);
478-
}
479-
480-
return old_refcount;
481-
} /* }}} */
482-
483463
static void spl_array_write_dimension_ex(int check_inherited, zend_object *object, zval *offset, zval *value) /* {{{ */
484464
{
485465
spl_array_object *intern = spl_array_from_obj(object);
@@ -503,19 +483,12 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
503483
}
504484

505485
Z_TRY_ADDREF_P(value);
506-
507-
uint32_t refcount = 0;
508486
if (!offset || Z_TYPE_P(offset) == IS_NULL) {
509487
ht = spl_array_get_hash_table(intern);
510488
if (UNEXPECTED(ht == intern->sentinel_array)) {
511489
return;
512490
}
513-
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
514491
zend_hash_next_index_insert(ht, value);
515-
516-
if (refcount) {
517-
spl_array_set_refcount(intern->is_child, ht, refcount);
518-
}
519492
return;
520493
}
521494

@@ -530,17 +503,13 @@ static void spl_array_write_dimension_ex(int check_inherited, zend_object *objec
530503
spl_hash_key_release(&key);
531504
return;
532505
}
533-
refcount = spl_array_set_refcount(intern->is_child, ht, 1);
506+
534507
if (key.key) {
535508
zend_hash_update_ind(ht, key.key, value);
536509
spl_hash_key_release(&key);
537510
} else {
538511
zend_hash_index_update(ht, key.h, value);
539512
}
540-
541-
if (refcount) {
542-
spl_array_set_refcount(intern->is_child, ht, refcount);
543-
}
544513
} /* }}} */
545514

546515
static void spl_array_write_dimension(zend_object *object, zval *offset, zval *value) /* {{{ */
@@ -570,8 +539,6 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
570539
}
571540

572541
ht = spl_array_get_hash_table(intern);
573-
uint32_t refcount = spl_array_set_refcount(intern->is_child, ht, 1);
574-
575542
if (key.key) {
576543
zval *data = zend_hash_find(ht, key.key);
577544
if (data) {
@@ -596,10 +563,6 @@ static void spl_array_unset_dimension_ex(int check_inherited, zend_object *objec
596563
} else {
597564
zend_hash_index_del(ht, key.h);
598565
}
599-
600-
if (refcount) {
601-
spl_array_set_refcount(intern->is_child, ht, refcount);
602-
}
603566
} /* }}} */
604567

605568
static void spl_array_unset_dimension(zend_object *object, zval *offset) /* {{{ */
@@ -973,15 +936,6 @@ static void spl_array_set_array(zval *object, spl_array_object *intern, zval *ar
973936
} else {
974937
//??? TODO: try to avoid array duplication
975938
ZVAL_ARR(&intern->array, zend_array_dup(Z_ARR_P(array)));
976-
977-
if (intern->is_child) {
978-
Z_TRY_DELREF(intern->bucket->val);
979-
/*
980-
* replace bucket->val with copied array, so the changes between
981-
* parent and child object can affect each other.
982-
*/
983-
ZVAL_COPY(&intern->bucket->val, &intern->array);
984-
}
985939
}
986940
} else {
987941
if (Z_OBJ_HT_P(array) == &spl_handler_ArrayObject) {
@@ -1854,15 +1808,6 @@ PHP_METHOD(RecursiveArrayIterator, hasChildren)
18541808
static void spl_instantiate_child_arg(zend_class_entry *pce, zval *retval, zval *arg1, zval *arg2) /* {{{ */
18551809
{
18561810
object_init_ex(retval, pce);
1857-
spl_array_object *new_intern = Z_SPLARRAY_P(retval);
1858-
/*
1859-
* set new_intern->is_child is true to indicate that the object was created by
1860-
* RecursiveArrayIterator::getChildren() method.
1861-
*/
1862-
new_intern->is_child = true;
1863-
1864-
/* find the bucket of parent object. */
1865-
new_intern->bucket = (Bucket *)((char *)(arg1) - XtOffsetOf(Bucket, val));;
18661811
zend_call_known_instance_method_with_2_params(pce->constructor, Z_OBJ_P(retval), NULL, arg1, arg2);
18671812
}
18681813
/* }}} */

‎ext/spl/tests/bug42654.phpt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,11 +110,11 @@ object(RecursiveArrayIterator)#%d (1) {
110110
[2]=>
111111
array(2) {
112112
[2]=>
113-
string(5) "alter"
113+
string(4) "val2"
114114
[3]=>
115115
array(1) {
116116
[3]=>
117-
string(5) "alter"
117+
string(4) "val3"
118118
}
119119
}
120120
[4]=>
@@ -129,11 +129,11 @@ object(RecursiveArrayIterator)#%d (1) {
129129
[2]=>
130130
array(2) {
131131
[2]=>
132-
string(5) "alter"
132+
string(4) "val2"
133133
[3]=>
134134
array(1) {
135135
[3]=>
136-
string(5) "alter"
136+
string(4) "val3"
137137
}
138138
}
139139
[4]=>
@@ -146,11 +146,11 @@ array(3) {
146146
[2]=>
147147
array(2) {
148148
[2]=>
149-
string(5) "alter"
149+
string(4) "val2"
150150
[3]=>
151151
array(1) {
152152
[3]=>
153-
string(5) "alter"
153+
string(4) "val3"
154154
}
155155
}
156156
[4]=>

‎ext/spl/tests/bug42654_2.phpt‎

Lines changed: 0 additions & 33 deletions
This file was deleted.

‎ext/spl/tests/gh10519.phpt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
--TEST--
22
Bug GH-10519 (Array Data Address Reference Issue)
3+
--XFAIL--
4+
The fix for this was bad
35
--FILE--
46
<?php
57
interface DataInterface extends JsonSerializable, RecursiveIterator, Iterator

‎ext/spl/tests/gh21499.phpt‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
GH-21499 (RecursiveArrayIterator getChildren UAF after parent free)
3+
--FILE--
4+
<?php
5+
$it = new RecursiveArrayIterator([[1]]);
6+
$child = $it->getChildren();
7+
unset($it);
8+
$child->__construct([2, 3]);
9+
var_dump($child->getArrayCopy());
10+
?>
11+
--EXPECT--
12+
array(2) {
13+
[0]=>
14+
int(2)
15+
[1]=>
16+
int(3)
17+
}

0 commit comments

Comments
 (0)