Skip to content

Commit 8dd0d10

Browse files
devnexendevnexen
committed
ext/spl: fix use-after-free in RecursiveIteratorIterator on reentrant rewind().
Guard hasChildren()/getChildren() against reentrant rewind()/next() the same way valid() was guarded. Follow-up to GH-22466, GH-22478.
1 parent 86d78cc commit 8dd0d10

3 files changed

Lines changed: 121 additions & 6 deletions

File tree

‎ext/spl/spl_iterators.c‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,15 +282,23 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv
282282
/* break; */
283283
/* TODO: Check this is correct */
284284
ZEND_FALLTHROUGH;
285-
case RS_TEST:
285+
case RS_TEST: {
286+
zend_object *sub_object = Z_OBJ(object->iterators[object->level].zobject);
287+
uint32_t prev_level = object->level;
288+
GC_ADDREF(sub_object);
286289
if (object->callHasChildren) {
287290
zend_call_method_with_0_params(Z_OBJ_P(zthis), object->ce, &object->callHasChildren, "callHasChildren", &retval);
288291
} else {
289292
zend_class_entry *ce = object->iterators[object->level].ce;
290-
zend_object *obj = Z_OBJ(object->iterators[object->level].zobject);
291293
zend_function **cache = &object->iterators[object->level].haschildren;
292294

293-
zend_call_method_with_0_params(obj, ce, cache, "haschildren", &retval);
295+
zend_call_method_with_0_params(sub_object, ce, cache, "haschildren", &retval);
296+
}
297+
bool reentered = object->level != prev_level || object->iterators[object->level].iterator != iterator;
298+
OBJ_RELEASE(sub_object);
299+
if (reentered) {
300+
zval_ptr_dtor(&retval);
301+
return;
294302
}
295303
if (EG(exception)) {
296304
if (!(object->flags & RIT_CATCH_GET_CHILD)) {
@@ -336,6 +344,7 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv
336344
}
337345
}
338346
return /* self */;
347+
}
339348
case RS_SELF:
340349
if (object->nextElement && (object->mode == RIT_SELF_FIRST || object->mode == RIT_CHILD_FIRST)) {
341350
zend_call_method_with_0_params(Z_OBJ_P(zthis), object->ce, &object->nextElement, "nextelement", NULL);
@@ -346,15 +355,23 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv
346355
object->iterators[object->level].state = RS_NEXT;
347356
}
348357
return /* self */;
349-
case RS_CHILD:
358+
case RS_CHILD: {
359+
zend_object *sub_object = Z_OBJ(object->iterators[object->level].zobject);
360+
uint32_t prev_level = object->level;
361+
GC_ADDREF(sub_object);
350362
if (object->callGetChildren) {
351363
zend_call_method_with_0_params(Z_OBJ_P(zthis), object->ce, &object->callGetChildren, "callGetChildren", &child);
352364
} else {
353365
zend_class_entry *ce = object->iterators[object->level].ce;
354-
zend_object *obj = Z_OBJ(object->iterators[object->level].zobject);
355366
zend_function **cache = &object->iterators[object->level].getchildren;
356367

357-
zend_call_method_with_0_params(obj, ce, cache, "getchildren", &child);
368+
zend_call_method_with_0_params(sub_object, ce, cache, "getchildren", &child);
369+
}
370+
bool reentered = object->level != prev_level || object->iterators[object->level].iterator != iterator;
371+
OBJ_RELEASE(sub_object);
372+
if (reentered) {
373+
zval_ptr_dtor(&child);
374+
return;
358375
}
359376

360377
if (EG(exception)) {
@@ -410,6 +427,7 @@ static void spl_recursive_it_move_forward_ex(spl_recursive_it_object *object, zv
410427
}
411428
}
412429
goto next_step;
430+
}
413431
}
414432
/* no more elements */
415433
if (object->level > 0) {

‎ext/spl/tests/recursiveiteratoriterator_rewind_during_getchildren.phpt‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
--TEST--
2+
RecursiveIteratorIterator: rewind() re-entered from getChildren() must not use-after-free
3+
--FILE--
4+
<?php
5+
// As with the hasChildren() case, a reentrant rewind() runs
6+
// spl_recursive_it_rewind_ex() and erealloc()s object->iterators down to one
7+
// element. The outer spl_recursive_it_move_forward_ex() frame is mid
8+
// getChildren() in RS_CHILD and still holds
9+
// cache = &object->iterators[object->level].getchildren and the cached
10+
// sub-object, both of which now dangle (spl_iterators.c:357).
11+
class RIt implements RecursiveIterator {
12+
public $data;
13+
public $pos = 0;
14+
public $rii;
15+
public $depth;
16+
public static bool $fired = false;
17+
function __construct(array $d, $depth = 0) { $this->data = $d; $this->depth = $depth; }
18+
function current(): mixed { return $this->data[$this->pos] ?? null; }
19+
function key(): mixed { return $this->pos; }
20+
function next(): void { $this->pos++; }
21+
function rewind(): void { $this->pos = 0; }
22+
function valid(): bool { return $this->pos < count($this->data); }
23+
function hasChildren(): bool { return is_array($this->current()); }
24+
function getChildren(): RecursiveIterator {
25+
if ($this->rii && $this->depth >= 1 && !self::$fired) {
26+
self::$fired = true;
27+
$this->rii->rewind();
28+
}
29+
$c = new RIt($this->current(), $this->depth + 1);
30+
$c->rii = $this->rii;
31+
return $c;
32+
}
33+
}
34+
35+
$root = new RIt([[[1, 2], 3], 4]);
36+
$rii = new RecursiveIteratorIterator($root, RecursiveIteratorIterator::SELF_FIRST);
37+
$root->rii = $rii;
38+
$seen = [];
39+
foreach ($rii as $v) {
40+
$seen[] = is_array($v) ? 'A' : $v;
41+
if (count($seen) > 30) { $seen[] = '...'; break; }
42+
}
43+
echo implode(' ', $seen), "\n";
44+
echo "done\n";
45+
?>
46+
--EXPECT--
47+
A A A A 1 2 3 4
48+
done

‎ext/spl/tests/recursiveiteratoriterator_rewind_during_haschildren.phpt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
RecursiveIteratorIterator: rewind() re-entered from hasChildren() must not use-after-free
3+
--FILE--
4+
<?php
5+
// A reentrant rewind() runs spl_recursive_it_rewind_ex(), which tears down every
6+
// active level and erealloc()s object->iterators back to a single element. The
7+
// outer spl_recursive_it_move_forward_ex() frame is mid hasChildren() in RS_TEST
8+
// and still holds cache = &object->iterators[object->level].haschildren plus the
9+
// cached sub-object, both of which now dangle (spl_iterators.c:293).
10+
class RIt implements RecursiveIterator {
11+
public $data;
12+
public $pos = 0;
13+
public $rii;
14+
public $depth;
15+
public static bool $fired = false;
16+
function __construct(array $d, $depth = 0) { $this->data = $d; $this->depth = $depth; }
17+
function current(): mixed { return $this->data[$this->pos] ?? null; }
18+
function key(): mixed { return $this->pos; }
19+
function next(): void { $this->pos++; }
20+
function rewind(): void { $this->pos = 0; }
21+
function valid(): bool { return $this->pos < count($this->data); }
22+
function hasChildren(): bool {
23+
if ($this->rii && $this->depth >= 1 && !self::$fired) {
24+
self::$fired = true;
25+
$this->rii->rewind();
26+
}
27+
return is_array($this->current());
28+
}
29+
function getChildren(): RecursiveIterator {
30+
$c = new RIt($this->current(), $this->depth + 1);
31+
$c->rii = $this->rii;
32+
return $c;
33+
}
34+
}
35+
36+
$root = new RIt([[[1, 2], 3], 4]);
37+
$rii = new RecursiveIteratorIterator($root, RecursiveIteratorIterator::SELF_FIRST);
38+
$root->rii = $rii;
39+
$seen = [];
40+
foreach ($rii as $v) {
41+
$seen[] = is_array($v) ? 'A' : $v;
42+
if (count($seen) > 30) { $seen[] = '...'; break; }
43+
}
44+
echo implode(' ', $seen), "\n";
45+
echo "done\n";
46+
?>
47+
--EXPECT--
48+
A A A 1 2 3 4
49+
done

0 commit comments

Comments
 (0)