From 5611f19b47b9d1d16fb5c30827d90d521601a7cc Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Fri, 26 Jun 2026 15:15:47 -0400 Subject: [PATCH] Fix use-after-free in RecursiveIteratorIterator on reentrant teardown spl_recursive_it_move_forward_ex() tears down the exhausted level after running its sub-iterator, but endChildren() and a sub-iterator's valid() can re-enter through $this->next() and tear that level down first. The no-more-elements branch then dtored a stale iterator pointer, and valid() kept running on a sub-iterator the reentrant call had already freed. Guard the teardown on the level's iterator being unchanged, and hold a reference on the sub-iterator across valid(). --- ext/spl/spl_iterators.c | 18 ++++++- ...ratoriterator_next_during_endchildren.phpt | 42 +++++++++++++++++ ...iveiteratoriterator_next_during_valid.phpt | 47 +++++++++++++++++++ 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt create mode 100644 ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c index db100d228341..cca678cd44dd 100644 --- a/ext/spl/spl_iterators.c +++ b/ext/spl/spl_iterators.c @@ -257,6 +257,10 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv zend_class_entry *ce; zval retval, child; zend_object_iterator *sub_iter; + zend_object *sub_object; + uint32_t prev_level; + zend_result valid_result; + bool reentered; SPL_FETCH_SUB_ITERATOR(iterator, object); @@ -275,7 +279,17 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv } ZEND_FALLTHROUGH; case RS_START: - if (iterator->funcs->valid(iterator) == FAILURE) { + sub_object = Z_OBJ(object->iterators[object->level].zobject); + prev_level = object->level; + GC_ADDREF(sub_object); + valid_result = iterator->funcs->valid(iterator); + reentered = object->level != prev_level + || object->iterators[object->level].iterator != iterator; + OBJ_RELEASE(sub_object); + if (reentered) { + return; + } + if (valid_result == FAILURE) { break; } object->iterators[object->level].state = RS_TEST; @@ -423,7 +437,7 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv } } } - if (object->level > 0) { + if (object->level > 0 && object->iterators[object->level].iterator == iterator) { zval garbage; ZVAL_COPY_VALUE(&garbage, &object->iterators[object->level].zobject); ZVAL_UNDEF(&object->iterators[object->level].zobject); diff --git a/ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt b/ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt new file mode 100644 index 000000000000..b3c80b2232a8 --- /dev/null +++ b/ext/spl/tests/recursiveiteratoriterator_next_during_endchildren.phpt @@ -0,0 +1,42 @@ +--TEST-- +RecursiveIteratorIterator: next() re-entered from endChildren() must not use-after-free +--FILE-- +data = $d; } + function current(): mixed { return $this->data[$this->pos] ?? null; } + function key(): mixed { return $this->pos; } + function next(): void { $this->pos++; } + function rewind(): void { $this->pos = 0; } + function valid(): bool { return $this->pos < count($this->data); } + function hasChildren(): bool { return is_array($this->current()); } + function getChildren(): RecursiveIterator { return new RIt($this->current()); } +} + +class ReenterRII extends RecursiveIteratorIterator { + public static int $fired = 0; + function endChildren(): void { + if (self::$fired++ === 0) { + $this->next(); + } + } +} + +$tree = [[[1, 2], 3], 4]; +$it = new ReenterRII(new RIt($tree), RecursiveIteratorIterator::SELF_FIRST); +$seen = []; +foreach ($it as $v) { + $seen[] = is_array($v) ? 'array' : $v; + if (count($seen) > 20) { + $seen[] = '...'; + break; + } +} +echo implode(' ', $seen), "\n"; +echo "done\n"; +?> +--EXPECT-- +array array 1 2 4 +done diff --git a/ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt b/ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt new file mode 100644 index 000000000000..a2dcf4695e36 --- /dev/null +++ b/ext/spl/tests/recursiveiteratoriterator_next_during_valid.phpt @@ -0,0 +1,47 @@ +--TEST-- +RecursiveIteratorIterator: next() re-entered from a sub-iterator valid() must not use-after-free +--FILE-- +data = $d; $this->depth = $depth; } + function current(): mixed { return $this->data[$this->pos] ?? null; } + function key(): mixed { return $this->pos; } + function next(): void { $this->pos++; } + function rewind(): void { $this->pos = 0; } + function valid(): bool { + if ($this->rii && $this->depth === 2 && $this->pos === count($this->data) && !self::$fired) { + self::$fired = true; + $this->rii->next(); + } + return $this->pos < count($this->data); + } + function hasChildren(): bool { return is_array($this->current()); } + function getChildren(): RecursiveIterator { + $c = new RIt($this->current(), $this->depth + 1); + $c->rii = $this->rii; + return $c; + } +} + +$root = new RIt([[[1, 2], 3], 4]); +$rii = new RecursiveIteratorIterator($root, RecursiveIteratorIterator::SELF_FIRST); +$root->rii = $rii; +$seen = []; +foreach ($rii as $v) { + $seen[] = is_array($v) ? 'array' : $v; + if (count($seen) > 20) { + $seen[] = '...'; + break; + } +} +echo implode(' ', $seen), "\n"; +echo "done\n"; +?> +--EXPECT-- +array array 1 2 3 4 +done