Fix GH-12192: SimpleXML infinite loop when getName() is called within…

… foreach This happens because getName() resets the iterator to the start because it overwrites the iterator data. We add a version of get_first_node that does not overwrite the iterator data. Closes GH-12193.
php · Sep 16, 2023 · 4d888cf · 4d888cf
1 parent 10f5a06
commit 4d888cf
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 8 deletions.
diff --git a/NEWS b/NEWS
@@ -11,6 +11,8 @@ PHP                                                                        NEWS
 
 - SimpleXML:
   . Fixed bug GH-12170 (Can't use xpath with comments in SimpleXML). (nielsdos)
+  . Fixed bug GH-12192 (SimpleXML infinite loop when getName() is called
+    within foreach). (nielsdos)
 
 28 Sep 2023, PHP 8.1.24
 

diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c
@@ -45,6 +45,7 @@ PHP_SXE_API zend_class_entry *sxe_get_element_class_entry(void) /* {{{ */
 
 static php_sxe_object* php_sxe_object_new(zend_class_entry *ce, zend_function *fptr_count);
 static xmlNodePtr php_sxe_reset_iterator(php_sxe_object *sxe, int use_data);
+static xmlNodePtr php_sxe_reset_iterator_no_clear_iter_data(php_sxe_object *sxe, int use_data);
 static xmlNodePtr php_sxe_iterator_fetch(php_sxe_object *sxe, xmlNodePtr node, int use_data);
 static void php_sxe_iterator_dtor(zend_object_iterator *iter);
 static int php_sxe_iterator_valid(zend_object_iterator *iter);
@@ -77,6 +78,7 @@ static void _node_as_zval(php_sxe_object *sxe, xmlNodePtr node, zval *value, SXE
 }
 /* }}} */
 
+/* Important: this overwrites the iterator data, if you wish to keep it use php_sxe_get_first_node_non_destructive() instead! */
 static xmlNodePtr php_sxe_get_first_node(php_sxe_object *sxe, xmlNodePtr node) /* {{{ */
 {
 	php_sxe_object *intern;
@@ -95,6 +97,15 @@ static xmlNodePtr php_sxe_get_first_node(php_sxe_object *sxe, xmlNodePtr node) /
 }
 /* }}} */
 
+static xmlNodePtr php_sxe_get_first_node_non_destructive(php_sxe_object *sxe, xmlNodePtr node)
+{
+	if (sxe && sxe->iter.type != SXE_ITER_NONE) {
+		return php_sxe_reset_iterator_no_clear_iter_data(sxe, false);
+	} else {
+		return node;
+	}
+}
+
 static inline int match_ns(php_sxe_object *sxe, xmlNodePtr node, xmlChar *name, int prefix) /* {{{ */
 {
 	if (name == NULL && (node->ns == NULL || node->ns->prefix == NULL)) {
@@ -1625,7 +1636,7 @@ PHP_METHOD(SimpleXMLElement, getName)
 	sxe = Z_SXEOBJ_P(ZEND_THIS);
 
 	GET_NODE(sxe, node);
-	node = php_sxe_get_first_node(sxe, node);
+	node = php_sxe_get_first_node_non_destructive(sxe, node);
 	if (node) {
 		namelen = xmlStrlen(node->name);
 		RETURN_STRINGL((char*)node->name, namelen);
@@ -2450,15 +2461,9 @@ static xmlNodePtr php_sxe_iterator_fetch(php_sxe_object *sxe, xmlNodePtr node, i
 }
 /* }}} */
 
-static xmlNodePtr php_sxe_reset_iterator(php_sxe_object *sxe, int use_data) /* {{{ */
+static xmlNodePtr php_sxe_reset_iterator_no_clear_iter_data(php_sxe_object *sxe, int use_data)
 {
 	xmlNodePtr node;
-
-	if (!Z_ISUNDEF(sxe->iter.data)) {
-		zval_ptr_dtor(&sxe->iter.data);
-		ZVAL_UNDEF(&sxe->iter.data);
-	}
-
 	GET_NODE(sxe, node)
 
 	if (node) {
@@ -2471,10 +2476,23 @@ static xmlNodePtr php_sxe_reset_iterator(php_sxe_object *sxe, int use_data) /* {
 			case SXE_ITER_ATTRLIST:
 				node = (xmlNodePtr) node->properties;
 		}
+		if (use_data) {
+			ZEND_ASSERT(Z_ISUNDEF(sxe->iter.data));
+		}
 		return php_sxe_iterator_fetch(sxe, node, use_data);
 	}
 	return NULL;
 }
+
+static xmlNodePtr php_sxe_reset_iterator(php_sxe_object *sxe, int use_data) /* {{{ */
+{
+	if (!Z_ISUNDEF(sxe->iter.data)) {
+		zval_ptr_dtor(&sxe->iter.data);
+		ZVAL_UNDEF(&sxe->iter.data);
+	}
+
+	return php_sxe_reset_iterator_no_clear_iter_data(sxe, use_data);
+}
 /* }}} */
 
 zend_object_iterator *php_sxe_get_iterator(zend_class_entry *ce, zval *object, int by_ref) /* {{{ */

diff --git a/ext/simplexml/tests/gh12192.phpt b/ext/simplexml/tests/gh12192.phpt
@@ -0,0 +1,37 @@
+--TEST--
+GH-12192 (SimpleXML infinite loop when getName() is called within foreach)
+--EXTENSIONS--
+simplexml
+--FILE--
+<?php
+
+$xml = "<root><a>1</a><a>2</a></root>";
+$xml = simplexml_load_string($xml);
+
+$a = $xml->a;
+
+foreach ($a as $test) {
+    echo "Iteration\n";
+    var_dump($a->key());
+    var_dump($a->getName());
+    var_dump((string) $test);
+}
+
+var_dump($a);
+
+?>
+--EXPECT--
+Iteration
+string(1) "a"
+string(1) "a"
+string(1) "1"
+Iteration
+string(1) "a"
+string(1) "a"
+string(1) "2"
+object(SimpleXMLElement)#2 (2) {
+  [0]=>
+  string(1) "1"
+  [1]=>
+  string(1) "2"
+}