From 7163218b1408b2f4185bd7bd18f8030823ff16b6 Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Sat, 8 Aug 2026 16:38:02 +0800 Subject: [PATCH 1/4] Fix GH-23120: DOMNode::isEqualNode stack overflow on deeply nested trees --- NEWS | 2 ++ ext/dom/node.c | 73 +++++++++++++++++++++++++++----------- ext/dom/tests/gh23120.phpt | 27 ++++++++++++++ 3 files changed, 82 insertions(+), 20 deletions(-) create mode 100644 ext/dom/tests/gh23120.phpt diff --git a/NEWS b/NEWS index ba832ef05888..3a33074d7eff 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,8 @@ PHP NEWS . Fixed bug GH-22447 (UAF at dom_objects_free_storage when setting an attribute node that collides by local name with a namespaced attribute). (David Carlier) + . Fixed bug GH-23120 (Stack overflow when comparing deeply nested DOM nodes + with DOMNode::isEqualNode()). (Weilin Du) - MBString: . Fixed bug GH-22779 (mb_strrpos() returns the wrong position for a negative diff --git a/ext/dom/node.c b/ext/dom/node.c index 81c80cb0c8ac..18ad83992452 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -1678,22 +1678,6 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other } \ return counter; \ } -#define PHP_DOM_DEFINE_LIST_EQUALITY_ORDERED_HELPER(type) \ - static bool PHP_DOM_FUNC_CAT(php_dom_node_list_equality_check_ordered, type)(const type *list1, const type *list2, bool spec_compliant) \ - { \ - size_t count = PHP_DOM_FUNC_CAT(php_dom_node_count_list_size, type)(list1); \ - if (count != PHP_DOM_FUNC_CAT(php_dom_node_count_list_size, type)(list2)) { \ - return false; \ - } \ - for (size_t i = 0; i < count; i++) { \ - if (!php_dom_node_is_equal_node((const xmlNode *) list1, (const xmlNode *) list2, spec_compliant)) { \ - return false; \ - } \ - list1 = list1->next; \ - list2 = list2->next; \ - } \ - return true; \ - } #define PHP_DOM_DEFINE_LIST_EQUALITY_UNORDERED_HELPER(type) \ static bool PHP_DOM_FUNC_CAT(php_dom_node_list_equality_check_unordered, type)(const type *list1, const type *list2, bool spec_compliant)\ { \ @@ -1717,7 +1701,6 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other PHP_DOM_DEFINE_LIST_COUNTER_HELPER(xmlNode) PHP_DOM_DEFINE_LIST_COUNTER_HELPER(xmlNs) -PHP_DOM_DEFINE_LIST_EQUALITY_ORDERED_HELPER(xmlNode) PHP_DOM_DEFINE_LIST_EQUALITY_UNORDERED_HELPER(xmlNode) PHP_DOM_DEFINE_LIST_EQUALITY_UNORDERED_HELPER(xmlNs) @@ -1730,7 +1713,15 @@ static bool php_dom_is_equal_attr(const xmlAttr *this_attr, const xmlAttr *other && php_dom_node_is_content_equal((const xmlNode *) this_attr, (const xmlNode *) other_attr); } -static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other, bool spec_compliant) +static zend_always_inline bool php_dom_node_has_ordered_children(const xmlNode *node) +{ + return node->type == XML_ELEMENT_NODE + || node->type == XML_DOCUMENT_FRAG_NODE + || node->type == XML_HTML_DOCUMENT_NODE + || node->type == XML_DOCUMENT_NODE; +} + +static bool php_dom_node_is_equal_node_without_children(const xmlNode *this, const xmlNode *other, bool spec_compliant) { ZEND_ASSERT(this != NULL); ZEND_ASSERT(other != NULL); @@ -1749,7 +1740,8 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other /* Check attributes first, then namespace declarations, then children */ && php_dom_node_list_equality_check_unordered_xmlNode((const xmlNode *) this->properties, (const xmlNode *) other->properties, spec_compliant) && (spec_compliant || php_dom_node_list_equality_check_unordered_xmlNs(this->nsDef, other->nsDef, false)) - && php_dom_node_list_equality_check_ordered_xmlNode(this->children, other->children, spec_compliant); + && php_dom_node_count_list_size_xmlNode(this->children) + == php_dom_node_count_list_size_xmlNode(other->children); } else if (this->type == XML_DTD_NODE) { /* Note: in the living spec entity declarations and notations are no longer compared because they're considered obsolete. */ const xmlDtd *this_dtd = (const xmlDtd *) this; @@ -1780,12 +1772,53 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other const xmlNs *other_ns = (const xmlNs *) other; return xmlStrEqual(this_ns->prefix, other_ns->prefix) && xmlStrEqual(this_ns->href, other_ns->href); } else if (this->type == XML_DOCUMENT_FRAG_NODE || this->type == XML_HTML_DOCUMENT_NODE || this->type == XML_DOCUMENT_NODE) { - return php_dom_node_list_equality_check_ordered_xmlNode(this->children, other->children, spec_compliant); + return php_dom_node_count_list_size_xmlNode(this->children) + == php_dom_node_count_list_size_xmlNode(other->children); } return false; } +static bool php_dom_node_list_equality_check_ordered_xmlNode(const xmlNode *list1, const xmlNode *list2, bool spec_compliant) +{ + size_t count = php_dom_node_count_list_size_xmlNode(list1); + if (count != php_dom_node_count_list_size_xmlNode(list2)) { + return false; + } + if (count == 0) { + return true; + } + + const xmlNode *base1 = list1->parent; + const xmlNode *base2 = list2->parent; + + /* Immediate child counts are checked by the node comparison below. This + * preserves the tree structure while walking both subtrees in tree order. */ + while (list1 != NULL && list2 != NULL) { + if (!php_dom_node_is_equal_node_without_children(list1, list2, spec_compliant)) { + return false; + } + + list1 = php_dom_next_in_tree_order(list1, base1); + list2 = php_dom_next_in_tree_order(list2, base2); + } + + return list1 == NULL && list2 == NULL; +} + +static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other, bool spec_compliant) +{ + if (!php_dom_node_is_equal_node_without_children(this, other, spec_compliant)) { + return false; + } + + if (php_dom_node_has_ordered_children(this)) { + return php_dom_node_list_equality_check_ordered_xmlNode(this->children, other->children, spec_compliant); + } + + return true; +} + /* {{{ URL: https://dom.spec.whatwg.org/#dom-node-isequalnode (for everything still in the living spec) * URL: https://www.w3.org/TR/2004/REC-DOM-Level-3-Core-20040407/DOM3-Core.html#core-Node3-isEqualNode (for old nodes removed from the living spec) Since: DOM Level 3 diff --git a/ext/dom/tests/gh23120.phpt b/ext/dom/tests/gh23120.phpt new file mode 100644 index 000000000000..89651bce5db2 --- /dev/null +++ b/ext/dom/tests/gh23120.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-23120 (Stack overflow when comparing deeply nested DOM nodes) +--EXTENSIONS-- +dom +--FILE-- +createElement('leaf', 'x'); + + for ($i = 0; $i < 100000; $i++) { + $parent = $doc->createElement('a'); + $parent->appendChild($node); + $node = $parent; + } + + $doc->appendChild($node); + return $doc; +} + +$doc1 = create_deep_document(); +$doc2 = create_deep_document(); + +var_dump($doc1->documentElement->isEqualNode($doc2->documentElement)); +?> +--EXPECT-- +bool(true) From fd8a0e430b115ec21060998b1dde615d1708f7fe Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Sat, 8 Aug 2026 17:52:14 +0800 Subject: [PATCH 2/4] feedback & fix CI --- ext/dom/node.c | 88 ++++++++++++++++---------------------- ext/dom/tests/gh23120.phpt | 21 +++++++-- 2 files changed, 55 insertions(+), 54 deletions(-) diff --git a/ext/dom/node.c b/ext/dom/node.c index 18ad83992452..62090dc837ac 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -1678,6 +1678,22 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other } \ return counter; \ } +#define PHP_DOM_DEFINE_LIST_EQUALITY_ORDERED_HELPER(type) \ + static bool PHP_DOM_FUNC_CAT(php_dom_node_list_equality_check_ordered, type)(const type *list1, const type *list2, bool spec_compliant) \ + { \ + size_t count = PHP_DOM_FUNC_CAT(php_dom_node_count_list_size, type)(list1); \ + if (count != PHP_DOM_FUNC_CAT(php_dom_node_count_list_size, type)(list2)) { \ + return false; \ + } \ + for (size_t i = 0; i < count; i++) { \ + if (!php_dom_node_is_equal_node((const xmlNode *) list1, (const xmlNode *) list2, spec_compliant)) { \ + return false; \ + } \ + list1 = list1->next; \ + list2 = list2->next; \ + } \ + return true; \ + } #define PHP_DOM_DEFINE_LIST_EQUALITY_UNORDERED_HELPER(type) \ static bool PHP_DOM_FUNC_CAT(php_dom_node_list_equality_check_unordered, type)(const type *list1, const type *list2, bool spec_compliant)\ { \ @@ -1701,6 +1717,7 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other PHP_DOM_DEFINE_LIST_COUNTER_HELPER(xmlNode) PHP_DOM_DEFINE_LIST_COUNTER_HELPER(xmlNs) +PHP_DOM_DEFINE_LIST_EQUALITY_ORDERED_HELPER(xmlNode) PHP_DOM_DEFINE_LIST_EQUALITY_UNORDERED_HELPER(xmlNode) PHP_DOM_DEFINE_LIST_EQUALITY_UNORDERED_HELPER(xmlNs) @@ -1713,19 +1730,25 @@ static bool php_dom_is_equal_attr(const xmlAttr *this_attr, const xmlAttr *other && php_dom_node_is_content_equal((const xmlNode *) this_attr, (const xmlNode *) other_attr); } -static zend_always_inline bool php_dom_node_has_ordered_children(const xmlNode *node) +static zend_always_inline bool php_dom_node_is_equal_node_check_stack_limit(void) { - return node->type == XML_ELEMENT_NODE - || node->type == XML_DOCUMENT_FRAG_NODE - || node->type == XML_HTML_DOCUMENT_NODE - || node->type == XML_DOCUMENT_NODE; +#ifdef ZEND_CHECK_STACK_LIMIT + return zend_call_stack_overflowed(EG(stack_limit)); +#else + return false; +#endif } -static bool php_dom_node_is_equal_node_without_children(const xmlNode *this, const xmlNode *other, bool spec_compliant) +static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other, bool spec_compliant) { ZEND_ASSERT(this != NULL); ZEND_ASSERT(other != NULL); + if (UNEXPECTED(php_dom_node_is_equal_node_check_stack_limit())) { + zend_throw_error(NULL, "Maximum call stack size reached. Infinite recursion?"); + return false; + } + if (this->type != other->type) { return false; } @@ -1740,8 +1763,7 @@ static bool php_dom_node_is_equal_node_without_children(const xmlNode *this, con /* Check attributes first, then namespace declarations, then children */ && php_dom_node_list_equality_check_unordered_xmlNode((const xmlNode *) this->properties, (const xmlNode *) other->properties, spec_compliant) && (spec_compliant || php_dom_node_list_equality_check_unordered_xmlNs(this->nsDef, other->nsDef, false)) - && php_dom_node_count_list_size_xmlNode(this->children) - == php_dom_node_count_list_size_xmlNode(other->children); + && php_dom_node_list_equality_check_ordered_xmlNode(this->children, other->children, spec_compliant); } else if (this->type == XML_DTD_NODE) { /* Note: in the living spec entity declarations and notations are no longer compared because they're considered obsolete. */ const xmlDtd *this_dtd = (const xmlDtd *) this; @@ -1772,51 +1794,10 @@ static bool php_dom_node_is_equal_node_without_children(const xmlNode *this, con const xmlNs *other_ns = (const xmlNs *) other; return xmlStrEqual(this_ns->prefix, other_ns->prefix) && xmlStrEqual(this_ns->href, other_ns->href); } else if (this->type == XML_DOCUMENT_FRAG_NODE || this->type == XML_HTML_DOCUMENT_NODE || this->type == XML_DOCUMENT_NODE) { - return php_dom_node_count_list_size_xmlNode(this->children) - == php_dom_node_count_list_size_xmlNode(other->children); - } - - return false; -} - -static bool php_dom_node_list_equality_check_ordered_xmlNode(const xmlNode *list1, const xmlNode *list2, bool spec_compliant) -{ - size_t count = php_dom_node_count_list_size_xmlNode(list1); - if (count != php_dom_node_count_list_size_xmlNode(list2)) { - return false; - } - if (count == 0) { - return true; - } - - const xmlNode *base1 = list1->parent; - const xmlNode *base2 = list2->parent; - - /* Immediate child counts are checked by the node comparison below. This - * preserves the tree structure while walking both subtrees in tree order. */ - while (list1 != NULL && list2 != NULL) { - if (!php_dom_node_is_equal_node_without_children(list1, list2, spec_compliant)) { - return false; - } - - list1 = php_dom_next_in_tree_order(list1, base1); - list2 = php_dom_next_in_tree_order(list2, base2); - } - - return list1 == NULL && list2 == NULL; -} - -static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other, bool spec_compliant) -{ - if (!php_dom_node_is_equal_node_without_children(this, other, spec_compliant)) { - return false; - } - - if (php_dom_node_has_ordered_children(this)) { return php_dom_node_list_equality_check_ordered_xmlNode(this->children, other->children, spec_compliant); } - return true; + return false; } /* {{{ URL: https://dom.spec.whatwg.org/#dom-node-isequalnode (for everything still in the living spec) @@ -1828,6 +1809,7 @@ static void dom_node_is_equal_node_common(INTERNAL_FUNCTION_PARAMETERS, bool mod zval *id, *node; xmlNodePtr otherp, nodep; dom_object *intern; + bool result; id = ZEND_THIS; ZEND_PARSE_PARAMETERS_START(1, 1) @@ -1850,7 +1832,11 @@ static void dom_node_is_equal_node_common(INTERNAL_FUNCTION_PARAMETERS, bool mod RETURN_BOOL(nodep == NULL && otherp == NULL); } - RETURN_BOOL(php_dom_node_is_equal_node(nodep, otherp, modern)); + result = php_dom_node_is_equal_node(nodep, otherp, modern); + if (UNEXPECTED(EG(exception))) { + RETURN_THROWS(); + } + RETURN_BOOL(result); } PHP_METHOD(DOMNode, isEqualNode) diff --git a/ext/dom/tests/gh23120.phpt b/ext/dom/tests/gh23120.phpt index 89651bce5db2..8c0cf7d5fbba 100644 --- a/ext/dom/tests/gh23120.phpt +++ b/ext/dom/tests/gh23120.phpt @@ -2,13 +2,24 @@ GH-23120 (Stack overflow when comparing deeply nested DOM nodes) --EXTENSIONS-- dom +--SKIPIF-- + +--INI-- +zend.max_allowed_stack_size=512K --FILE-- createElement('leaf', 'x'); - for ($i = 0; $i < 100000; $i++) { + for ($i = 0; $i < 10000; $i++) { $parent = $doc->createElement('a'); $parent->appendChild($node); $node = $parent; @@ -21,7 +32,11 @@ function create_deep_document(): DOMDocument { $doc1 = create_deep_document(); $doc2 = create_deep_document(); -var_dump($doc1->documentElement->isEqualNode($doc2->documentElement)); +try { + var_dump($doc1->documentElement->isEqualNode($doc2->documentElement)); +} catch (\Error $e) { + echo $e::class, ": ", $e->getMessage(), "\n"; +} ?> --EXPECT-- -bool(true) +Error: Maximum call stack size reached. Infinite recursion? From a579755fcbe7777290ace956701c8ab59d5db287 Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Sat, 8 Aug 2026 19:36:31 +0800 Subject: [PATCH 3/4] EG gate --- ext/dom/node.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ext/dom/node.c b/ext/dom/node.c index 62090dc837ac..dd9202949402 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -1745,7 +1745,9 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other ZEND_ASSERT(other != NULL); if (UNEXPECTED(php_dom_node_is_equal_node_check_stack_limit())) { - zend_throw_error(NULL, "Maximum call stack size reached. Infinite recursion?"); + if (!EG(exception)) { + zend_throw_error(NULL, "Maximum call stack size reached. Infinite recursion?"); + } return false; } From 7b2e2c99225cd4bf58c8ea201572bc94cb166273 Mon Sep 17 00:00:00 2001 From: Weilin Du Date: Sat, 8 Aug 2026 23:33:40 +0800 Subject: [PATCH 4/4] adjust error message --- ext/dom/node.c | 2 +- ext/dom/tests/gh23120.phpt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/dom/node.c b/ext/dom/node.c index dd9202949402..a42dfedc32a5 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -1746,7 +1746,7 @@ static bool php_dom_node_is_equal_node(const xmlNode *this, const xmlNode *other if (UNEXPECTED(php_dom_node_is_equal_node_check_stack_limit())) { if (!EG(exception)) { - zend_throw_error(NULL, "Maximum call stack size reached. Infinite recursion?"); + zend_throw_error(NULL, "Maximum call stack size reached."); } return false; } diff --git a/ext/dom/tests/gh23120.phpt b/ext/dom/tests/gh23120.phpt index 8c0cf7d5fbba..511314d9cbba 100644 --- a/ext/dom/tests/gh23120.phpt +++ b/ext/dom/tests/gh23120.phpt @@ -39,4 +39,4 @@ try { } ?> --EXPECT-- -Error: Maximum call stack size reached. Infinite recursion? +Error: Maximum call stack size reached.