From 09390e73661034996732b23db103c96d6d54d06b Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 18 Aug 2026 05:54:53 +0100 Subject: [PATCH 1/2] ext/dom: php_dom_free_templated_content() use-after-free. Fix #23334 The fragment was freed with xmlFreeNode(), assuming no userland reference into template contents could exist. getElementById() reaches those nodes, so freeing the template element left dangling proxies. The children now go through php_libxml_node_free_list(), and the fragment is freed only when it has no proxy; otherwise its parent link is cleared so it is freed with the last reference. --- ext/dom/private_data.c | 12 +++++--- .../modern/html/interactions/gh23334.phpt | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) create mode 100644 ext/dom/tests/modern/html/interactions/gh23334.phpt diff --git a/ext/dom/private_data.c b/ext/dom/private_data.c index 66b3f4f87f29..d908ad329251 100644 --- a/ext/dom/private_data.c +++ b/ext/dom/private_data.c @@ -72,9 +72,7 @@ void php_dom_private_data_destroy(php_dom_private_data *data) static void php_dom_free_templated_content(php_dom_private_data *private_data, xmlNodePtr base) { - /* Note: it's not possible to obtain a userland reference to these yet, so we can just free them without worrying - * about their proxies. - * Note 2: it's possible to have nested template content. */ + /* Note: it's possible to have nested template content. */ if (zend_hash_num_elements(private_data->template_fragments) > 0) { /* There's more templated content, try to free it. */ @@ -88,7 +86,13 @@ static void php_dom_free_templated_content(php_dom_private_data *private_data, x } } - xmlFreeNode(base); + php_libxml_node_free_list(base->children); + + if (!base->_private) { + xmlFreeNode(base); + } else { + base->parent = NULL; + } } void php_dom_add_templated_content(php_dom_private_data *private_data, const xmlNode *template_node, xmlNodePtr fragment) diff --git a/ext/dom/tests/modern/html/interactions/gh23334.phpt b/ext/dom/tests/modern/html/interactions/gh23334.phpt new file mode 100644 index 000000000000..82a86b6c1bf8 --- /dev/null +++ b/ext/dom/tests/modern/html/interactions/gh23334.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-23334 (UAF when template contents are freed while userland references remain) +--EXTENSIONS-- +dom +--FILE-- +
'; + +// A descendant of the template contents outlives the template element. +$doc = Dom\HTMLDocument::createFromString($html, LIBXML_NOERROR); +$inner = $doc->getElementById('inner'); +$tpl = $doc->getElementById('tpl'); +$tpl->remove(); +unset($tpl); +var_dump($inner->textContent); +var_dump($inner->parentNode); + +// The template contents fragment itself outlives the template element. +$doc = Dom\HTMLDocument::createFromString($html, LIBXML_NOERROR); +$frag = $doc->getElementById('inner')->parentNode; +var_dump($frag::class); +$doc->getElementById('host')->innerHTML = ''; +var_dump($frag->childNodes->length); +?> +--EXPECT-- +string(5) "HELLO" +NULL +string(20) "Dom\DocumentFragment" +int(0) From 43310cbeb60cbd3f35e36049df43a962e5d52643 Mon Sep 17 00:00:00 2001 From: David Carlier Date: Tue, 18 Aug 2026 07:10:24 +0100 Subject: [PATCH 2/2] Template contents leaking into the host element's tree. Upward walks followed the fragment's parent link to its host, placing template contents in the host document instead of rooting them at the fragment. They now stop at a fragment via php_dom_parent_node(), except the cycle check which the spec resolves against host-including ancestors. --- ext/dom/node.c | 22 +++---- ext/dom/parentnode/css_selectors.c | 2 +- ext/dom/php_dom.h | 5 ++ .../html/interactions/gh23334_tree.phpt | 57 +++++++++++++++++++ 4 files changed, 74 insertions(+), 12 deletions(-) create mode 100644 ext/dom/tests/modern/html/interactions/gh23334_tree.phpt diff --git a/ext/dom/node.c b/ext/dom/node.c index a42dfedc32a5..686c16f34b2f 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -66,7 +66,7 @@ bool php_dom_is_node_connected(const xmlNode *node) if (node->type == XML_DOCUMENT_NODE || node->type == XML_HTML_DOCUMENT_NODE) { return true; } - node = node->parent; + node = php_dom_parent_node(node); } while (node != NULL); return false; } @@ -244,7 +244,7 @@ static zend_result dom_node_parent_get(dom_object *obj, zval *retval, bool only_ { DOM_PROP_NODE(xmlNodePtr, nodep, obj); - xmlNodePtr nodeparent = nodep->parent; + xmlNodePtr nodeparent = php_dom_parent_node(nodep); if (!nodeparent || (only_element && nodeparent->type != XML_ELEMENT_NODE)) { ZVAL_NULL(retval); return SUCCESS; @@ -2412,7 +2412,7 @@ static bool dom_node_contains(xmlNodePtr thisp, xmlNodePtr otherp) if (otherp == thisp) { return true; } - otherp = otherp->parent; + otherp = php_dom_parent_node(otherp); } while (otherp); return false; @@ -2470,7 +2470,7 @@ PHP_METHOD(Dom_Node, contains) PHP_METHOD(DOMNode, getRootNode) { zval *id; - xmlNodePtr thisp; + xmlNodePtr thisp, tmp; dom_object *intern; /* Unused now because we don't support the shadow DOM nodes. Options only influence shadow DOM nodes. */ zval *options; @@ -2482,8 +2482,8 @@ PHP_METHOD(DOMNode, getRootNode) DOM_GET_THIS_OBJ(thisp, id, xmlNodePtr, intern); - while (thisp->parent) { - thisp = thisp->parent; + while ((tmp = php_dom_parent_node(thisp))) { + thisp = tmp; } DOM_RET_OBJ(thisp, intern); @@ -2559,9 +2559,9 @@ static void dom_node_compare_document_position(INTERNAL_FUNCTION_PARAMETERS, zen } bool node2_is_ancestor_of_node1 = false; size_t node1_depth = 0; - xmlNodePtr node1_root = node1; - while (node1_root->parent) { - node1_root = node1_root->parent; + xmlNodePtr node1_root = node1, tmp; + while ((tmp = php_dom_parent_node(node1_root))) { + node1_root = tmp; if (node1_root == node2) { node2_is_ancestor_of_node1 = true; } @@ -2570,8 +2570,8 @@ static void dom_node_compare_document_position(INTERNAL_FUNCTION_PARAMETERS, zen bool node1_is_ancestor_of_node2 = false; size_t node2_depth = 0; xmlNodePtr node2_root = node2; - while (node2_root->parent) { - node2_root = node2_root->parent; + while ((tmp = php_dom_parent_node(node2_root))) { + node2_root = tmp; if (node2_root == node1) { node1_is_ancestor_of_node2 = true; } diff --git a/ext/dom/parentnode/css_selectors.c b/ext/dom/parentnode/css_selectors.c index 37f9d698e856..358a425db729 100644 --- a/ext/dom/parentnode/css_selectors.c +++ b/ext/dom/parentnode/css_selectors.c @@ -200,7 +200,7 @@ static const xmlNode *dom_query_closest( ret = current; break; } - current = current->parent; + current = php_dom_parent_node(current); } } diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index 13f49879bb38..8fe5d57e7f49 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -247,6 +247,11 @@ xmlNodePtr dom_nodelist_iter_start_first_child(xmlNodePtr nodep); __ptr = (__prtype)((php_libxml_node_ptr *)__intern->ptr)->node; \ } +static zend_always_inline xmlNodePtr php_dom_parent_node(const xmlNode *nodep) +{ + return nodep->type == XML_DOCUMENT_FRAG_NODE ? NULL : nodep->parent; +} + static zend_always_inline bool php_dom_is_cache_tag_stale_from_doc_ptr(const php_libxml_cache_tag *cache_tag, const php_libxml_ref_obj *doc_ptr) { ZEND_ASSERT(doc_ptr != NULL); diff --git a/ext/dom/tests/modern/html/interactions/gh23334_tree.phpt b/ext/dom/tests/modern/html/interactions/gh23334_tree.phpt new file mode 100644 index 000000000000..6e6b666d7fdf --- /dev/null +++ b/ext/dom/tests/modern/html/interactions/gh23334_tree.phpt @@ -0,0 +1,57 @@ +--TEST-- +GH-23334 (template contents must not be part of the host element's tree) +--EXTENSIONS-- +dom +--FILE-- +
', + LIBXML_NOERROR +); +$tpl = $doc->getElementById('tpl'); +$inner = $doc->getElementById('inner'); +$frag = $inner->parentNode; + +var_dump($frag::class); +var_dump($frag->parentNode); +var_dump($frag->parentElement); +var_dump($frag->getRootNode() === $frag); +var_dump($inner->getRootNode() === $frag); +var_dump($frag->isConnected); +var_dump($inner->isConnected); + +var_dump($frag->contains($inner)); +var_dump($tpl->contains($inner)); +var_dump($doc->body->contains($inner)); + +var_dump($inner->closest('b')?->nodeName); +var_dump($inner->closest('#host')); +var_dump($inner->closest('body')); + +$pos = $tpl->compareDocumentPosition($inner); +var_dump((bool) ($pos & 1)); +var_dump((bool) ($pos & 0x10)); + +try { + $frag->appendChild($tpl); +} catch (\Throwable $e) { + echo $e::class, ': ', $e->getMessage(), PHP_EOL; +} +?> +--EXPECT-- +string(20) "Dom\DocumentFragment" +NULL +NULL +bool(true) +bool(true) +bool(false) +bool(false) +bool(true) +bool(false) +bool(false) +string(1) "B" +NULL +NULL +bool(true) +bool(false) +DOMException: Hierarchy Request Error