Skip to content

ext/spl: fix use-after-free in RecursiveIteratorIterator on reentrant… #22496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
devnexen wants to merge 1 commit into
base: master
Choose a base branch
from
+184 −16
Open

ext/spl: fix use-after-free in RecursiveIteratorIterator on reentrant… #22496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 42 additions & 16 deletions ext/spl/spl_iterators.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ typedef struct _spl_recursive_it_object {
int flags;
int max_depth;
bool in_iteration;
bool in_progress;
zend_function *beginIteration;
zend_function *endIteration;
zend_function *callHasChildren;
Expand Down Expand Up @@ -255,6 +256,11 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv
zend_result valid_result;
bool reentered;

if (object->in_progress) {
zend_throw_error(NULL, "Cannot advance a RecursiveIteratorIterator from within hasChildren() or getChildren()");
return;
}

SPL_FETCH_SUB_ITERATOR(iterator, object);

while (!EG(exception)) {
Expand Down Expand Up @@ -291,15 +297,22 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv
/* TODO: Check this is correct */
ZEND_FALLTHROUGH;
case RS_TEST:
if (object->callHasChildren) {
zend_call_method_with_0_params(Z_OBJ_P(zthis), object->ce, &object->callHasChildren, "callHasChildren", &retval);
} else {
zend_class_entry *ce = object->iterators[object->level].ce;
zend_object *obj = Z_OBJ(object->iterators[object->level].zobject);
zend_function **cache = &object->iterators[object->level].haschildren;
object->in_progress = true;
zend_try {
if (object->callHasChildren) {
zend_call_method_with_0_params(Z_OBJ_P(zthis), object->ce, &object->callHasChildren, "callHasChildren", &retval);
} else {
zend_class_entry *ce = object->iterators[object->level].ce;
zend_object *obj = Z_OBJ(object->iterators[object->level].zobject);
zend_function **cache = &object->iterators[object->level].haschildren;

zend_call_method_with_0_params(obj, ce, cache, "haschildren", &retval);
}
zend_call_method_with_0_params(obj, ce, cache, "haschildren", &retval);
}
} zend_catch {
object->in_progress = false;
zend_bailout();
} zend_end_try();
object->in_progress = false;
if (EG(exception)) {
if (!(object->flags & RIT_CATCH_GET_CHILD)) {
object->iterators[object->level].state = RS_NEXT;
Expand Down Expand Up @@ -355,15 +368,22 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv
}
return /* self */;
case RS_CHILD:
if (object->callGetChildren) {
zend_call_method_with_0_params(Z_OBJ_P(zthis), object->ce, &object->callGetChildren, "callGetChildren", &child);
} else {
zend_class_entry *ce = object->iterators[object->level].ce;
zend_object *obj = Z_OBJ(object->iterators[object->level].zobject);
zend_function **cache = &object->iterators[object->level].getchildren;
object->in_progress = true;
zend_try {
if (object->callGetChildren) {
zend_call_method_with_0_params(Z_OBJ_P(zthis), object->ce, &object->callGetChildren, "callGetChildren", &child);
} else {
zend_class_entry *ce = object->iterators[object->level].ce;
zend_object *obj = Z_OBJ(object->iterators[object->level].zobject);
zend_function **cache = &object->iterators[object->level].getchildren;

zend_call_method_with_0_params(obj, ce, cache, "getchildren", &child);
}
zend_call_method_with_0_params(obj, ce, cache, "getchildren", &child);
}
} zend_catch {
object->in_progress = false;
zend_bailout();
} zend_end_try();
object->in_progress = false;

if (EG(exception)) {
if (!(object->flags & RIT_CATCH_GET_CHILD)) {
Expand Down Expand Up @@ -449,6 +469,11 @@ static void spl_recursive_it_rewind_ex(spl_recursive_it_object *object, zval *zt
{
zend_object_iterator *sub_iter;

if (object->in_progress) {
zend_throw_error(NULL, "Cannot rewind a RecursiveIteratorIterator from within hasChildren() or getChildren()");
return;
}

SPL_FETCH_SUB_ITERATOR(sub_iter, object);

while (object->level) {
Expand Down Expand Up @@ -623,6 +648,7 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
intern->flags = (int)flags;
intern->max_depth = -1;
intern->in_iteration = false;
intern->in_progress = false;
intern->ce = Z_OBJCE_P(object);

intern->beginIteration = zend_hash_str_find_ptr(&intern->ce->function_table, "beginiteration", sizeof("beginiteration") - 1);
Expand Down
45 changes: 45 additions & 0 deletions ext/spl/tests/recursiveiteratoriterator_next_during_getchildren.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
--TEST--
RecursiveIteratorIterator: next() re-entered from getChildren() throws
--FILE--
<?php
// getChildren() must not advance the outer iterator either: a reentrant next()
// runs spl_recursive_it_move_forward_ex() while the outer frame is mid
// getChildren(), so it is guarded the same way as the reentrant rewind().
class RIt implements RecursiveIterator {
public $data;
public $pos = 0;
public $rii;
public $depth;
public static bool $fired = false;
function __construct(array $d, $depth = 0) { $this->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 { return $this->pos < count($this->data); }
function hasChildren(): bool { return is_array($this->current()); }
function getChildren(): RecursiveIterator {
if ($this->rii && $this->depth >= 1 && !self::$fired) {
self::$fired = true;
$this->rii->next();
}
$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;
try {
foreach ($rii as $v) {
}
} catch (\Error $e) {
echo $e->getMessage(), "\n";
}
echo "done\n";
?>
--EXPECT--
Cannot advance a RecursiveIteratorIterator from within hasChildren() or getChildren()
done
48 changes: 48 additions & 0 deletions ext/spl/tests/recursiveiteratoriterator_rewind_during_getchildren.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
--TEST--
RecursiveIteratorIterator: rewind() re-entered from getChildren() throws
--FILE--
<?php
// As with the hasChildren() case, a reentrant rewind() used to run
// spl_recursive_it_rewind_ex() and erealloc() object->iterators down to one
// element while the outer spl_recursive_it_move_forward_ex() frame was mid
// getChildren() in RS_CHILD, leaving its cached sub-object and iterators[]
// dangling. getChildren() returns an iterator for the *current* item, so
// rewinding the outer iterator from inside it now throws.
class RIt implements RecursiveIterator {
public $data;
public $pos = 0;
public $rii;
public $depth;
public static bool $fired = false;
function __construct(array $d, $depth = 0) { $this->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 { return $this->pos < count($this->data); }
function hasChildren(): bool { return is_array($this->current()); }
function getChildren(): RecursiveIterator {
if ($this->rii && $this->depth >= 1 && !self::$fired) {
self::$fired = true;
$this->rii->rewind();
}
$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;
try {
foreach ($rii as $v) {
}
} catch (\Error $e) {
echo $e->getMessage(), "\n";
}
echo "done\n";
?>
--EXPECT--
Cannot rewind a RecursiveIteratorIterator from within hasChildren() or getChildren()
done
49 changes: 49 additions & 0 deletions ext/spl/tests/recursiveiteratoriterator_rewind_during_haschildren.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
--TEST--
RecursiveIteratorIterator: rewind() re-entered from hasChildren() throws
--FILE--
<?php
// A reentrant rewind() used to run spl_recursive_it_rewind_ex(), tearing down
// every active level and erealloc()ing object->iterators back to a single
// element while the outer spl_recursive_it_move_forward_ex() frame was still mid
// hasChildren(), leaving the cached sub-object and iterators[] dangling.
// hasChildren() must not mutate the iteration state, so this now throws instead.
class RIt implements RecursiveIterator {
public $data;
public $pos = 0;
public $rii;
public $depth;
public static bool $fired = false;
function __construct(array $d, $depth = 0) { $this->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 { return $this->pos < count($this->data); }
function hasChildren(): bool {
if ($this->rii && $this->depth >= 1 && !self::$fired) {
self::$fired = true;
$this->rii->rewind();
}
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;
try {
foreach ($rii as $v) {
}
} catch (\Error $e) {
echo $e->getMessage(), "\n";
}
echo "done\n";
?>
--EXPECT--
Cannot rewind a RecursiveIteratorIterator from within hasChildren() or getChildren()
done
Loading