Fix phpGH-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 phpGH-16156.

Author: ndossche

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

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

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>