From b9ae73eee97e2b43e7766ecf2eab2655c9e3e481 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv@gmail.com>
Date: Thu, 15 Jul 2021 13:09:18 +0200
Subject: [PATCH] Fix RecursiveIteratorIterator segfault for invalid aggregate

The code was assuming that the returned value is an object.
Reuse the logic from IteratorIterator.
---
 ext/spl/spl_iterators.c                       | 42 ++++++++++++-------
 ...iveIteratorIterator_invalid_aggregate.phpt | 20 +++++++++
 2 files changed, 47 insertions(+), 15 deletions(-)
 create mode 100644 ext/spl/tests/RecursiveIteratorIterator_invalid_aggregate.phpt

diff --git a/ext/spl/spl_iterators.c b/ext/spl/spl_iterators.c
index 40538cda8ee..251f5999a38 100644
--- a/ext/spl/spl_iterators.c
+++ b/ext/spl/spl_iterators.c
@@ -462,6 +462,24 @@ static zend_object_iterator *spl_recursive_it_get_iterator(zend_class_entry *ce,
 	return (zend_object_iterator*)iterator;
 }
 
+static int spl_get_iterator_from_aggregate(zval *retval, zend_class_entry *ce, zend_object *obj) {
+	zend_function **getiterator_cache =
+		ce->iterator_funcs_ptr ? &ce->iterator_funcs_ptr->zf_new_iterator : NULL;
+	zend_call_method_with_0_params(obj, ce, getiterator_cache, "getiterator", retval);
+	if (EG(exception)) {
+		return FAILURE;
+	}
+	if (Z_TYPE_P(retval) != IS_OBJECT
+			|| !instanceof_function(Z_OBJCE_P(retval), zend_ce_traversable)) {
+		zend_throw_exception_ex(spl_ce_LogicException, 0,
+			"%s::getIterator() must return an object that implements Traversable",
+			ZSTR_VAL(ce->name));
+		zval_ptr_dtor(retval);
+		return FAILURE;
+	}
+	return SUCCESS;
+}
+
 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)
 {
 	zval *object = ZEND_THIS;
@@ -485,9 +503,10 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
 
 			zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
 			if (instanceof_function(Z_OBJCE_P(iterator), zend_ce_aggregate)) {
-				zend_function **getiterator_cache = Z_OBJCE_P(iterator)->iterator_funcs_ptr
-					? &Z_OBJCE_P(iterator)->iterator_funcs_ptr->zf_new_iterator : NULL;
-				zend_call_method_with_0_params(Z_OBJ_P(iterator), Z_OBJCE_P(iterator), getiterator_cache, "getiterator", &aggregate_retval);
+				if (spl_get_iterator_from_aggregate(
+						&aggregate_retval, Z_OBJCE_P(iterator), Z_OBJ_P(iterator)) == FAILURE) {
+					RETURN_THROWS();
+				}
 				iterator = &aggregate_retval;
 			} else {
 				Z_ADDREF_P(iterator);
@@ -510,9 +529,10 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
 
 			zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
 			if (instanceof_function(Z_OBJCE_P(iterator), zend_ce_aggregate)) {
-				zend_function **getiterator_cache = Z_OBJCE_P(iterator)->iterator_funcs_ptr
-					? &Z_OBJCE_P(iterator)->iterator_funcs_ptr->zf_new_iterator : NULL;
-				zend_call_method_with_0_params(Z_OBJ_P(iterator), Z_OBJCE_P(iterator), getiterator_cache, "getiterator", &aggregate_retval);
+				if (spl_get_iterator_from_aggregate(
+						&aggregate_retval, Z_OBJCE_P(iterator), Z_OBJ_P(iterator)) == FAILURE) {
+					RETURN_THROWS();
+				}
 				iterator = &aggregate_retval;
 			} else {
 				Z_ADDREF_P(iterator);
@@ -1340,15 +1360,7 @@ static spl_dual_it_object* spl_dual_it_construct(INTERNAL_FUNCTION_PARAMETERS, z
 					ce = ce_cast;
 				}
 				if (instanceof_function(ce, zend_ce_aggregate)) {
-					zend_function **getiterator_cache =
-						ce->iterator_funcs_ptr ? &ce->iterator_funcs_ptr->zf_new_iterator : NULL;
-					zend_call_method_with_0_params(Z_OBJ_P(zobject), ce, getiterator_cache, "getiterator", &retval);
-					if (EG(exception)) {
-						zval_ptr_dtor(&retval);
-						return NULL;
-					}
-					if (Z_TYPE(retval) != IS_OBJECT || !instanceof_function(Z_OBJCE(retval), zend_ce_traversable)) {
-						zend_throw_exception_ex(spl_ce_LogicException, 0, "%s::getIterator() must return an object that implements Traversable", ZSTR_VAL(ce->name));
+					if (spl_get_iterator_from_aggregate(&retval, ce, Z_OBJ_P(zobject)) == FAILURE) {
 						return NULL;
 					}
 					zobject = &retval;
diff --git a/ext/spl/tests/RecursiveIteratorIterator_invalid_aggregate.phpt b/ext/spl/tests/RecursiveIteratorIterator_invalid_aggregate.phpt
new file mode 100644
index 00000000000..2d29149dd4b
--- /dev/null
+++ b/ext/spl/tests/RecursiveIteratorIterator_invalid_aggregate.phpt
@@ -0,0 +1,20 @@
+--TEST--
+RecursiveIteratorIterator constructor should thrown if IteratorAggregate does not return Iterator
+--FILE--
+<?php
+
+class MyIteratorAggregate implements IteratorAggregate {
+    function getIterator() {
+        return null;
+    }
+}
+
+try {
+    new RecursiveIteratorIterator(new MyIteratorAggregate);
+} catch (LogicException $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+MyIteratorAggregate::getIterator() must return an object that implements Traversable