From 81dc49313ca184bc7ab5d75a3422c455792509a7 Mon Sep 17 00:00:00 2001
From: Niels Dossche <7771979+nielsdos@users.noreply.github.com>
Date: Tue, 1 Oct 2024 20:50:56 +0200
Subject: [PATCH] Fix GH-16151: Assertion failure in ext/dom/parentnode/tree.c

Unfortunately, old DOM allows attributes to be used as parent nodes.
Only text nodes and entities are allowed as children for these types of
nodes, because that's the constraint DOM and libxml give us.
---
 ext/dom/node.c             | 73 ++++++++++++++++++--------------------
 ext/dom/tests/gh16151.phpt | 35 ++++++++++++++++++
 2 files changed, 70 insertions(+), 38 deletions(-)
 create mode 100644 ext/dom/tests/gh16151.phpt

diff --git a/ext/dom/node.c b/ext/dom/node.c
index c80f9c3333c6c..32f348e06b674 100644
--- a/ext/dom/node.c
+++ b/ext/dom/node.c
@@ -843,6 +843,39 @@ static xmlNodePtr _php_dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib,
 }
 /* }}} */
 
+static bool dom_node_check_legacy_insertion_validity(xmlNodePtr parentp, xmlNodePtr child, bool stricterror)
+{
+	if (dom_node_is_read_only(parentp) == SUCCESS ||
+		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
+		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
+		return false;
+	}
+
+	if (dom_hierarchy(parentp, child) == FAILURE) {
+		php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
+		return false;
+	}
+
+	if (child->doc != parentp->doc && child->doc != NULL) {
+		php_dom_throw_error(WRONG_DOCUMENT_ERR, stricterror);
+		return false;
+	}
+
+	if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
+		/* TODO Drop Warning? */
+		php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
+		return false;
+	}
+
+	/* In old DOM only text nodes can be added as children to attributes. */
+	if (parentp->type == XML_ATTRIBUTE_NODE && child->type != XML_TEXT_NODE && child->type != XML_ENTITY_REF_NODE) {
+		php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
+		return false;
+	}
+
+	return true;
+}
+
 /* {{{ URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-952280727
 Since:
 */
@@ -870,25 +903,7 @@ PHP_METHOD(DOMNode, insertBefore)
 
 	stricterror = dom_get_strict_error(intern->document);
 
-	if (dom_node_is_read_only(parentp) == SUCCESS ||
-		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
-		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
-		RETURN_FALSE;
-	}
-
-	if (dom_hierarchy(parentp, child) == FAILURE) {
-		php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
-		RETURN_FALSE;
-	}
-
-	if (child->doc != parentp->doc && child->doc != NULL) {
-		php_dom_throw_error(WRONG_DOCUMENT_ERR, stricterror);
-		RETURN_FALSE;
-	}
-
-	if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
-		/* TODO Drop Warning? */
-		php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
+	if (!dom_node_check_legacy_insertion_validity(parentp, child, stricterror)) {
 		RETURN_FALSE;
 	}
 
@@ -1170,25 +1185,7 @@ PHP_METHOD(DOMNode, appendChild)
 
 	stricterror = dom_get_strict_error(intern->document);
 
-	if (dom_node_is_read_only(nodep) == SUCCESS ||
-		(child->parent != NULL && dom_node_is_read_only(child->parent) == SUCCESS)) {
-		php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
-		RETURN_FALSE;
-	}
-
-	if (dom_hierarchy(nodep, child) == FAILURE) {
-		php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
-		RETURN_FALSE;
-	}
-
-	if (!(child->doc == NULL || child->doc == nodep->doc)) {
-		php_dom_throw_error(WRONG_DOCUMENT_ERR, stricterror);
-		RETURN_FALSE;
-	}
-
-	if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
-		/* TODO Drop Warning? */
-		php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
+	if (!dom_node_check_legacy_insertion_validity(nodep, child, stricterror)) {
 		RETURN_FALSE;
 	}
 
diff --git a/ext/dom/tests/gh16151.phpt b/ext/dom/tests/gh16151.phpt
new file mode 100644
index 0000000000000..e11d3df4a56bb
--- /dev/null
+++ b/ext/dom/tests/gh16151.phpt
@@ -0,0 +1,35 @@
+--TEST--
+GH-16151 (Assertion failure in ext/dom/parentnode/tree.c)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$element = new DOMElement("N", "W", "y");
+$attr = new DOMAttr("c" , "n");
+$doc = new DOMDocument();
+$doc->appendChild($element);
+$element->setAttributeNodeNS($attr);
+
+try {
+    $attr->insertBefore(new DOMComment("h"));
+} catch (DOMException $e) {
+    echo $e->getMessage(), "\n";
+}
+try {
+    $attr->appendChild(new DOMComment("h"));
+} catch (DOMException $e) {
+    echo $e->getMessage(), "\n";
+}
+
+$attr->insertBefore($doc->createEntityReference('amp'));
+$attr->appendChild($doc->createEntityReference('amp'));
+
+echo $doc->saveXML();
+
+?>
+--EXPECT--
+Hierarchy Request Error
+Hierarchy Request Error
+<?xml version="1.0"?>
+<N xmlns="y" c="n&amp;&amp;">W</N>