From 066d18f2e8ff01371ed8afb3e00b92a08ff39bd1 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.

Closes GH-16156.
---
 NEWS                       |  2 ++
 ext/dom/node.c             | 73 ++++++++++++++++++--------------------
 ext/dom/tests/gh16151.phpt | 35 ++++++++++++++++++
 3 files changed, 72 insertions(+), 38 deletions(-)
 create mode 100644 ext/dom/tests/gh16151.phpt

diff --git a/NEWS b/NEWS
index 0acebdf586585..13e5f49ae7cd8 100644
--- a/NEWS
+++ b/NEWS
@@ -20,6 +20,8 @@ PHP                                                                        NEWS
 - DOM:
   . Fixed bug GH-16039 (Segmentation fault (access null pointer) in
     ext/dom/parentnode/tree.c). (nielsdos)
+  . Fixed bug GH-16151 (Assertion failure in ext/dom/parentnode/tree.c).
+    (nielsdos)
 
 - LDAP:
   . Fixed bug GH-16032 (Various NULL pointer dereferencements in
diff --git a/ext/dom/node.c b/ext/dom/node.c
index c80f9c3333c6c..bb80408f2689f 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 and entity 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>