Fix use-after-free in RecursiveIteratorIterator on reentry#22466
Conversation
move_forward_ex() caches the active sub-iterator, then calls the inner iterator's move_forward(), which can re-enter userland. A next() that rewinds or advances the RecursiveIteratorIterator frees that sub-iterator, and the following validity check then reads freed memory. Re-fetch the sub-iterator after the call, the same way the no-more-elements branch already re-checks the level after endChildren().
| zend_clear_exception(); | ||
| } | ||
| } | ||
| iterator = object->iterators[object->level].iterator; |
There was a problem hiding this comment.
Since endChildren()/valid() are also reentry points, it might be worth dtoring object->iterators[object->level].iterator at line 431 with a new test wdyt ?
There was a problem hiding this comment.
Confirmed, and it's a separate pre-existing UAF (reproduces on base without this hunk). endChildren() re-entering via $this->next() frees the level's sub-iterator and pops the level inside the reentrant call, then the outer frame's zend_iterator_dtor(iterator) at line 432 frees the stale local again. rewind() reentry is already safe: it unwinds to level 0 and the if (object->level > 0) re-check catches it.
So it needs more than reloading object->iterators[object->level].iterator there: the reentrant next() already tore the level down, so the outer frame has to notice and skip the dtor/decrement. I'll fix it in a follow-up with its own test rather than widen this PR.
2 participants
A RecursiveIterator whose next() calls rewind() (or next()) on the RecursiveIteratorIterator wrapping it triggers a use-after-free: the move-forward loop caches the current sub-iterator, the inner move_forward() re-enters userland, the reentrant rewind() frees that sub-iterator, and the following validity check then reads freed memory. Re-fetching the sub-iterator after the inner call closes the window, mirroring the level re-check the no-more-elements branch already does after endChildren().