From 78a5e023ab9cc9c28c2d4d3248543561e34b2054 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 17 Oct 2024 21:12:31 -0700 Subject: [PATCH 1/2] GH-15911: prevent appending an AppendIterator to itself --- NEWS | 2 ++ ext/spl/spl_iterators.c | 5 +++++ ext/spl/tests/gh15911.phpt | 15 +++++++++++++++ 3 files changed, 22 insertions(+) create mode 100644 ext/spl/tests/gh15911.phpt diff --git a/NEWS b/NEWS index 103b778aacf47..fa7d93eccbf6d 100644 --- a/NEWS +++ b/NEWS @@ -76,6 +76,8 @@ PHP NEWS (ilutov) . Fixed bug GH-16479 (Use-after-free in SplObjectStorage::setInfo()). (ilutov) . Fixed bug GH-16478 (Use-after-free in SplFixedArray::unset()). (ilutov) + . Fixed bug GH-15911 (Infinite recursion in AppendIterator::append()). + (DanielEScherzer) - Standard: . Fixed bug GH-16293 (Failed assertion when throwing in assert() callback with diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index 976d29283b965..ee203825e0659 100644 --- a/ext/spl/spl_iterators.c +++ b/ext/spl/spl_iterators.c @@ -2961,6 +2961,11 @@ PHP_METHOD(AppendIterator, append) SPL_FETCH_AND_CHECK_DUAL_IT(intern, ZEND_THIS); + if (Z_OBJ_P(ZEND_THIS) == Z_OBJ_P(it)) { + zend_argument_value_error(1, "must be different than the iterator being appended to"); + RETURN_THROWS(); + } + if (intern->u.append.iterator->funcs->valid(intern->u.append.iterator) == SUCCESS && spl_dual_it_valid(intern) != SUCCESS) { spl_array_iterator_append(&intern->u.append.zarrayit, it); intern->u.append.iterator->funcs->move_forward(intern->u.append.iterator); diff --git a/ext/spl/tests/gh15911.phpt b/ext/spl/tests/gh15911.phpt new file mode 100644 index 0000000000000..b27e727e6e93c --- /dev/null +++ b/ext/spl/tests/gh15911.phpt @@ -0,0 +1,15 @@ +--TEST-- +GH-15911 (Infinite recursion when appending AppendIterator to itself) +--FILE-- +append($iterator); +?> +--EXPECTF-- +Fatal error: Uncaught ValueError: AppendIterator::append(): Argument #1 ($iterator) must be different than the iterator being appended to in %s:%d +Stack trace: +#0 %s(%d): AppendIterator->append(Object(AppendIterator)) +#1 {main} + thrown in %s on line %d From a9f48a1724c10ee9894532ee8a87c1cc006fe9d8 Mon Sep 17 00:00:00 2001 From: Daniel Scherzer Date: Thu, 17 Oct 2024 22:48:54 -0700 Subject: [PATCH 2/2] Prevert appending iterator containing self --- ext/spl/spl_array.c | 14 ++++++++++++++ ext/spl/spl_array.h | 1 + ext/spl/spl_iterators.c | 11 +++++++++++ ext/spl/tests/gh15911_included.phpt | 17 +++++++++++++++++ .../tests/{gh15911.phpt => gh15911_self.phpt} | 2 +- 5 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 ext/spl/tests/gh15911_included.phpt rename ext/spl/tests/{gh15911.phpt => gh15911_self.phpt} (91%) diff --git a/ext/spl/spl_array.c b/ext/spl/spl_array.c index dbe110f7c40da..33fcae3abd1f4 100644 --- a/ext/spl/spl_array.c +++ b/ext/spl/spl_array.c @@ -100,6 +100,20 @@ static inline HashTable *spl_array_get_hash_table(spl_array_object* intern) { /* } /* }}} */ +bool spl_array_iterator_contains(zval *object, zval *to_find) { + zend_object *obj_to_find = Z_OBJ_P(to_find); + spl_array_object *intern = Z_SPLARRAY_P(object); + // call spl_array_get_hash_table($40) + HashTable *ht = spl_array_get_hash_table(intern); + zval *iter = NULL; + ZEND_HASH_FOREACH_VAL(ht, iter) { + if (Z_OBJ_P(iter) == obj_to_find) { + return true; + } + } ZEND_HASH_FOREACH_END(); + return false; +} + static inline bool spl_array_is_object(spl_array_object *intern) /* {{{ */ { while (intern->ar_flags & SPL_ARRAY_USE_OTHER) { diff --git a/ext/spl/spl_array.h b/ext/spl/spl_array.h index e3ea2d3a546e7..f6e9eba847c8d 100644 --- a/ext/spl/spl_array.h +++ b/ext/spl/spl_array.h @@ -41,5 +41,6 @@ PHP_MINIT_FUNCTION(spl_array); extern void spl_array_iterator_append(zval *object, zval *append_value); extern void spl_array_iterator_key(zval *object, zval *return_value); +extern bool spl_array_iterator_contains(zval *object, zval *to_find); #endif /* SPL_ARRAY_H */ diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index ee203825e0659..9bf7dce4a2563 100644 --- a/ext/spl/spl_iterators.c +++ b/ext/spl/spl_iterators.c @@ -2965,6 +2965,17 @@ PHP_METHOD(AppendIterator, append) zend_argument_value_error(1, "must be different than the iterator being appended to"); RETURN_THROWS(); } + if (instanceof_function(Z_OBJCE_P(it), spl_ce_AppendIterator)) { + // Make sure that the iterator being appended doesn't contain the + // iterator being appended to, GH-15911 + // call spl_dual_it_from_obj(it.value.obj) + spl_dual_it_object *other_iterator = Z_SPLDUAL_IT_P(it); + ZEND_ASSERT(other_iterator->dit_type == DIT_AppendIterator); + if (spl_array_iterator_contains(&other_iterator->u.append.zarrayit, ZEND_THIS)) { + zend_argument_value_error(1, "must not contain the iterator being appended to"); + RETURN_THROWS(); + } + } if (intern->u.append.iterator->funcs->valid(intern->u.append.iterator) == SUCCESS && spl_dual_it_valid(intern) != SUCCESS) { spl_array_iterator_append(&intern->u.append.zarrayit, it); diff --git a/ext/spl/tests/gh15911_included.phpt b/ext/spl/tests/gh15911_included.phpt new file mode 100644 index 0000000000000..8a852a4ee6f95 --- /dev/null +++ b/ext/spl/tests/gh15911_included.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-15911 (Appending AppendIterator containing AppendIterator to the contained AppendIterator) +--FILE-- +append( $iterator ); +$iterator->append( $other ); + +?> +--EXPECTF-- +Fatal error: Uncaught ValueError: AppendIterator::append(): Argument #1 ($iterator) must not contain the iterator being appended to in %s:%d +Stack trace: +#0 %s(%d): AppendIterator->append(Object(AppendIterator)) +#1 {main} + thrown in %s on line %d diff --git a/ext/spl/tests/gh15911.phpt b/ext/spl/tests/gh15911_self.phpt similarity index 91% rename from ext/spl/tests/gh15911.phpt rename to ext/spl/tests/gh15911_self.phpt index b27e727e6e93c..f470ef84df76f 100644 --- a/ext/spl/tests/gh15911.phpt +++ b/ext/spl/tests/gh15911_self.phpt @@ -3,7 +3,7 @@ GH-15911 (Infinite recursion when appending AppendIterator to itself) --FILE-- append($iterator); ?>