Skip to content

Commit b9ae73e

Browse files
committed
Fix RecursiveIteratorIterator segfault for invalid aggregate
The code was assuming that the returned value is an object. Reuse the logic from IteratorIterator.
1 parent c0a1ef3 commit b9ae73e

File tree

2 files changed

+47
-15
lines changed

2 files changed

+47
-15
lines changed

ext/spl/spl_iterators.c

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,24 @@ static zend_object_iterator *spl_recursive_it_get_iterator(zend_class_entry *ce,
462462
return (zend_object_iterator*)iterator;
463463
}
464464

465+
static int spl_get_iterator_from_aggregate(zval *retval, zend_class_entry *ce, zend_object *obj) {
466+
zend_function **getiterator_cache =
467+
ce->iterator_funcs_ptr ? &ce->iterator_funcs_ptr->zf_new_iterator : NULL;
468+
zend_call_method_with_0_params(obj, ce, getiterator_cache, "getiterator", retval);
469+
if (EG(exception)) {
470+
return FAILURE;
471+
}
472+
if (Z_TYPE_P(retval) != IS_OBJECT
473+
|| !instanceof_function(Z_OBJCE_P(retval), zend_ce_traversable)) {
474+
zend_throw_exception_ex(spl_ce_LogicException, 0,
475+
"%s::getIterator() must return an object that implements Traversable",
476+
ZSTR_VAL(ce->name));
477+
zval_ptr_dtor(retval);
478+
return FAILURE;
479+
}
480+
return SUCCESS;
481+
}
482+
465483
static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_class_entry *ce_base, zend_class_entry *ce_inner, recursive_it_it_type rit_type)
466484
{
467485
zval *object = ZEND_THIS;
@@ -485,9 +503,10 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
485503

486504
zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
487505
if (instanceof_function(Z_OBJCE_P(iterator), zend_ce_aggregate)) {
488-
zend_function **getiterator_cache = Z_OBJCE_P(iterator)->iterator_funcs_ptr
489-
? &Z_OBJCE_P(iterator)->iterator_funcs_ptr->zf_new_iterator : NULL;
490-
zend_call_method_with_0_params(Z_OBJ_P(iterator), Z_OBJCE_P(iterator), getiterator_cache, "getiterator", &aggregate_retval);
506+
if (spl_get_iterator_from_aggregate(
507+
&aggregate_retval, Z_OBJCE_P(iterator), Z_OBJ_P(iterator)) == FAILURE) {
508+
RETURN_THROWS();
509+
}
491510
iterator = &aggregate_retval;
492511
} else {
493512
Z_ADDREF_P(iterator);
@@ -510,9 +529,10 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
510529

511530
zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
512531
if (instanceof_function(Z_OBJCE_P(iterator), zend_ce_aggregate)) {
513-
zend_function **getiterator_cache = Z_OBJCE_P(iterator)->iterator_funcs_ptr
514-
? &Z_OBJCE_P(iterator)->iterator_funcs_ptr->zf_new_iterator : NULL;
515-
zend_call_method_with_0_params(Z_OBJ_P(iterator), Z_OBJCE_P(iterator), getiterator_cache, "getiterator", &aggregate_retval);
532+
if (spl_get_iterator_from_aggregate(
533+
&aggregate_retval, Z_OBJCE_P(iterator), Z_OBJ_P(iterator)) == FAILURE) {
534+
RETURN_THROWS();
535+
}
516536
iterator = &aggregate_retval;
517537
} else {
518538
Z_ADDREF_P(iterator);
@@ -1340,15 +1360,7 @@ static spl_dual_it_object* spl_dual_it_construct(INTERNAL_FUNCTION_PARAMETERS, z
13401360
ce = ce_cast;
13411361
}
13421362
if (instanceof_function(ce, zend_ce_aggregate)) {
1343-
zend_function **getiterator_cache =
1344-
ce->iterator_funcs_ptr ? &ce->iterator_funcs_ptr->zf_new_iterator : NULL;
1345-
zend_call_method_with_0_params(Z_OBJ_P(zobject), ce, getiterator_cache, "getiterator", &retval);
1346-
if (EG(exception)) {
1347-
zval_ptr_dtor(&retval);
1348-
return NULL;
1349-
}
1350-
if (Z_TYPE(retval) != IS_OBJECT || !instanceof_function(Z_OBJCE(retval), zend_ce_traversable)) {
1351-
zend_throw_exception_ex(spl_ce_LogicException, 0, "%s::getIterator() must return an object that implements Traversable", ZSTR_VAL(ce->name));
1363+
if (spl_get_iterator_from_aggregate(&retval, ce, Z_OBJ_P(zobject)) == FAILURE) {
13521364
return NULL;
13531365
}
13541366
zobject = &retval;
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
RecursiveIteratorIterator constructor should thrown if IteratorAggregate does not return Iterator
3+
--FILE--
4+
<?php
5+
6+
class MyIteratorAggregate implements IteratorAggregate {
7+
function getIterator() {
8+
return null;
9+
}
10+
}
11+
12+
try {
13+
new RecursiveIteratorIterator(new MyIteratorAggregate);
14+
} catch (LogicException $e) {
15+
echo $e->getMessage(), "\n";
16+
}
17+
18+
?>
19+
--EXPECT--
20+
MyIteratorAggregate::getIterator() must return an object that implements Traversable

0 commit comments

Comments
 (0)