ext/dom: php_dom_free_templated_content() use-after-free.

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;
 		}

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);
 		}
 	}
 

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);

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)

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--
+<?php
+$html = '<!doctype html><body><div id="host"><template id="tpl"><div id="inner">HELLO</div></template></div>';
+
+// 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)

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--
+<?php
+$doc = Dom\HTMLDocument::createFromString(
+    '<!doctype html><body><div id="host"><template id="tpl"><b id="inner">X</b></template></div>',
+    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