Skip to content

Fix use-after-free in RecursiveIteratorIterator on reentrant teardown#22478

Closed
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/spl-rii-reentrant-teardown-uaf
Closed

Fix use-after-free in RecursiveIteratorIterator on reentrant teardown#22478
iliaal wants to merge 1 commit into
php:PHP-8.4from
iliaal:fix/spl-rii-reentrant-teardown-uaf

Conversation

@iliaal

@iliaal iliaal commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Follow-up to the endChildren()/valid() reentry devnexen raised reviewing #22466. Both new cases reproduce under ASAN. This overlaps #22466 in spl_recursive_it_move_forward_ex(), so whichever lands first, the other takes a trivial rebase.

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().
@iliaal iliaal closed this in efbf473 Jun 27, 2026
devnexen added a commit to devnexen/php-src that referenced this pull request Jun 28, 2026
hasChildren() and getChildren() must not mutate the iteration state:
hasChildren() is a predicate and getChildren() returns an iterator for the
*current* item. A reentrant rewind()/next() from within either ran
spl_recursive_it_rewind_ex()/move_forward_ex(), which tore down the active
levels and erealloc()ed object->iterators under the outer move_forward frame,
leaving its cached sub-object and iterators[] dangling (use-after-free).

Track that we are inside such a call and throw an Error if rewind()/next()
re-enters, instead of silently coping with it. The guard is reset across a
bailout so a fatal inside the callback cannot leave the iterator wedged.

Follow-up to phpGH-22466, phpGH-22478.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants