Fix #70359 and #78577

ext/dom/element.c

@@ -150,6 +150,7 @@ int dom_element_schema_type_info_read(dom_object *obj, zval *retval)
 
 /* }}} */
 
+/* Note: the object returned is not necessarily a node, but can be an attribute or a namespace declaration. */
 static xmlNodePtr dom_get_dom1_attribute(xmlNodePtr elem, xmlChar *name) /* {{{ */
 {
 	int len;
@@ -376,25 +377,13 @@ PHP_METHOD(DOMElement, getAttributeNode)
 	}
 
 	if (attrp->type == XML_NAMESPACE_DECL) {
-		xmlNsPtr curns;
-		xmlNodePtr nsparent;
-
-		nsparent = attrp->_private;
-		curns = xmlNewNs(NULL, attrp->name, NULL);
-		if (attrp->children) {
-			curns->prefix = xmlStrdup((xmlChar *) attrp->children);
-		}
-		if (attrp->children) {
-			attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *) attrp->children, attrp->name);
-		} else {
-			attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *)"xmlns", attrp->name);
-		}
-		attrp->type = XML_NAMESPACE_DECL;
-		attrp->parent = nsparent;
-		attrp->ns = curns;
+		xmlNsPtr original = (xmlNsPtr) attrp;
+		/* Keep parent alive, because we're a fake child. */
+		GC_ADDREF(&intern->std);
+		(void) php_dom_create_fake_namespace_decl(nodep, original, return_value, intern);
+	} else {
+		DOM_RET_OBJ((xmlNodePtr) attrp, &ret, intern);
 	}
-
-	DOM_RET_OBJ((xmlNodePtr) attrp, &ret, intern);
 }
 /* }}} end dom_element_get_attribute_node */
 

ext/dom/php_dom.c

@@ -61,6 +61,7 @@ PHP_DOM_EXPORT zend_class_entry *dom_namespace_node_class_entry;
 
 zend_object_handlers dom_object_handlers;
 zend_object_handlers dom_nnodemap_object_handlers;
+zend_object_handlers dom_object_namespace_node_handlers;
 #ifdef LIBXML_XPATH_ENABLED
 zend_object_handlers dom_xpath_object_handlers;
 #endif
@@ -86,6 +87,9 @@ static HashTable dom_xpath_prop_handlers;
 #endif
 /* }}} */
 
+static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type);
+static void dom_object_namespace_node_free_storage(zend_object *object);
+
 typedef int (*dom_read_t)(dom_object *obj, zval *retval);
 typedef int (*dom_write_t)(dom_object *obj, zval *newval);
 
@@ -570,6 +574,10 @@ PHP_MINIT_FUNCTION(dom)
 	dom_nnodemap_object_handlers.read_dimension = dom_nodelist_read_dimension;
 	dom_nnodemap_object_handlers.has_dimension = dom_nodelist_has_dimension;
 
+	memcpy(&dom_object_namespace_node_handlers, &dom_object_handlers, sizeof(zend_object_handlers));
+	dom_object_namespace_node_handlers.offset = XtOffsetOf(dom_object_namespace_node, dom.std);
+	dom_object_namespace_node_handlers.free_obj = dom_object_namespace_node_free_storage;
+
 	zend_hash_init(&classes, 0, NULL, NULL, 1);
 
 	dom_domexception_class_entry = register_class_DOMException(zend_ce_exception);
@@ -604,7 +612,7 @@ PHP_MINIT_FUNCTION(dom)
 	zend_hash_add_ptr(&classes, dom_node_class_entry->name, &dom_node_prop_handlers);
 
 	dom_namespace_node_class_entry = register_class_DOMNameSpaceNode();
-	dom_namespace_node_class_entry->create_object = dom_objects_new;
+	dom_namespace_node_class_entry->create_object = dom_objects_namespace_node_new;
 
 	zend_hash_init(&dom_namespace_node_prop_handlers, 0, NULL, dom_dtor_prop_handler, 1);
 	dom_register_prop_handler(&dom_namespace_node_prop_handlers, "nodeName", sizeof("nodeName")-1, dom_node_node_name_read, NULL);
@@ -1001,10 +1009,8 @@ void dom_namednode_iter(dom_object *basenode, int ntype, dom_object *intern, xml
 }
 /* }}} */
 
-static dom_object* dom_objects_set_class(zend_class_entry *class_type) /* {{{ */
+static void dom_objects_set_class_ex(zend_class_entry *class_type, dom_object *intern)
 {
-	dom_object *intern = zend_object_alloc(sizeof(dom_object), class_type);
-
 	zend_class_entry *base_class = class_type;
 	while ((base_class->type != ZEND_INTERNAL_CLASS || base_class->info.internal.module->module_number != dom_module_entry.module_number) && base_class->parent != NULL) {
 		base_class = base_class->parent;
@@ -1014,10 +1020,14 @@ static dom_object* dom_objects_set_class(zend_class_entry *class_type) /* {{{ */
 
 	zend_object_std_init(&intern->std, class_type);
 	object_properties_init(&intern->std, class_type);
+}
 
+static dom_object* dom_objects_set_class(zend_class_entry *class_type)
+{
+	dom_object *intern = zend_object_alloc(sizeof(dom_object), class_type);
+	dom_objects_set_class_ex(class_type, intern);
 	return intern;
 }
-/* }}} */
 
 /* {{{ dom_objects_new */
 zend_object *dom_objects_new(zend_class_entry *class_type)
@@ -1028,6 +1038,25 @@ zend_object *dom_objects_new(zend_class_entry *class_type)
 }
 /* }}} */
 
+static zend_object *dom_objects_namespace_node_new(zend_class_entry *class_type)
+{
+	dom_object_namespace_node *intern = zend_object_alloc(sizeof(dom_object_namespace_node), class_type);
+	dom_objects_set_class_ex(class_type, &intern->dom);
+	intern->dom.std.handlers = &dom_object_namespace_node_handlers;
+	return &intern->dom.std;
+}
+
+static void dom_object_namespace_node_free_storage(zend_object *object)
+{
+	dom_object_namespace_node *intern = php_dom_namespace_node_obj_from_obj(object);
+	if (intern->parent_intern != NULL) {
+		zval tmp;
+		ZVAL_OBJ(&tmp, &intern->parent_intern->std);
+		zval_ptr_dtor(&tmp);
+	}
+	dom_objects_free_storage(object);
+}
+
 #ifdef LIBXML_XPATH_ENABLED
 /* {{{ zend_object dom_xpath_objects_new(zend_class_entry *class_type) */
 zend_object *dom_xpath_objects_new(zend_class_entry *class_type)
@@ -1550,6 +1579,28 @@ xmlNsPtr dom_get_nsdecl(xmlNode *node, xmlChar *localName) {
 }
 /* }}} end dom_get_nsdecl */
 
+/* Note: Assumes the additional lifetime was already added in the caller. */
+xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern)
+{
+	xmlNodePtr attrp;
+	xmlNsPtr curns = xmlNewNs(NULL, original->href, NULL);
+	if (original->prefix) {
+		curns->prefix = xmlStrdup(original->prefix);
+		attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *) original->prefix, original->href);
+	} else {
+		attrp = xmlNewDocNode(nodep->doc, NULL, (xmlChar *)"xmlns", original->href);
+	}
+	attrp->type = XML_NAMESPACE_DECL;
+	attrp->parent = nodep;
+	attrp->ns = curns;
+
+	php_dom_create_object(attrp, return_value, parent_intern);
+	/* This object must exist, because we just created an object for it via php_dom_create_object(). */
+	dom_object *obj = ((php_libxml_node_ptr *)attrp->_private)->_private;
+	php_dom_namespace_node_obj_from_obj(&obj->std)->parent_intern = parent_intern;
+	return attrp;
+}
+
 static zval *dom_nodelist_read_dimension(zend_object *object, zval *offset, int type, zval *rv) /* {{{ */
 {
 	zval offset_copy;

ext/dom/php_dom.h

@@ -93,6 +93,18 @@ typedef struct {
 	HashPosition pos;
 } php_dom_iterator;
 
+typedef struct {
+	/* This may be a fake object that isn't actually in the children list of the parent.
+	 * This is because some namespace declaration nodes aren't stored on the parent in libxml2, so we have to fake it.
+	 * We could use a zval for this, but since this is always going to be an object let's save space... */
+	dom_object *parent_intern;
+	dom_object dom;
+} dom_object_namespace_node;
+
+static inline dom_object_namespace_node *php_dom_namespace_node_obj_from_obj(zend_object *obj) {
+	return (dom_object_namespace_node*)((char*)(obj) - XtOffsetOf(dom_object_namespace_node, dom.std));
+}
+
 #include "domexception.h"
 
 dom_object *dom_object_get_data(xmlNodePtr obj);
@@ -126,6 +138,7 @@ xmlNode *php_dom_libxml_hash_iter(xmlHashTable *ht, int index);
 xmlNode *php_dom_libxml_notation_iter(xmlHashTable *ht, int index);
 zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref);
 void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece, zend_class_entry *ce);
+xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern);
 
 void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc);
 void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc);

ext/dom/tests/bug70359.phpt

@@ -0,0 +1,83 @@
+--TEST--
+Bug #70359 (print_r() on DOMAttr causes Segfault in php_libxml_node_free_list())
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+echo "-- Test without parent --\n";
+
+$dom = new DOMDocument();
+$dom->loadXML(<<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:xsi="fooooooooooooooooooooo"/>
+XML);
+$spaceNode = $dom->documentElement->getAttributeNode('xmlns');
+print_r($spaceNode);
+
+echo "-- Test with parent and non-ns attribute --\n";
+
+$dom = new DOMDocument();
+$dom->loadXML(<<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
+<url xmlns:xsi="fooooooooooooooooooooo" myattrib="bar"/>
+</urlset>
+XML);
+$spaceNode = $dom->documentElement->firstElementChild->getAttributeNode('myattrib');
+var_dump($spaceNode->nodeType);
+var_dump($spaceNode->nodeValue);
+
+$dom->documentElement->firstElementChild->remove();
+try {
+    print_r($spaceNode->parentNode);
+} catch (\Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+echo "-- Test with parent and ns attribute --\n";
+
+$dom = new DOMDocument();
+$dom->loadXML(<<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">
+<url xmlns:xsi="fooooooooooooooooooooo" myattrib="bar"/>
+</urlset>
+XML);
+$spaceNode = $dom->documentElement->firstElementChild->getAttributeNode('xmlns:xsi');
+print_r($spaceNode);
+
+$dom->documentElement->firstElementChild->remove();
+var_dump($spaceNode->parentNode->nodeName); // Shouldn't crash
+
+?>
+--EXPECT--
+-- Test without parent --
+DOMNameSpaceNode Object
+(
+    [nodeName] => xmlns
+    [nodeValue] => http://www.sitemaps.org/schemas/sitemap/0.9
+    [nodeType] => 18
+    [prefix] => 
+    [localName] => xmlns
+    [namespaceURI] => http://www.sitemaps.org/schemas/sitemap/0.9
+    [ownerDocument] => (object value omitted)
+    [parentNode] => (object value omitted)
+)
+-- Test with parent and non-ns attribute --
+int(2)
+string(3) "bar"
+Couldn't fetch DOMAttr. Node no longer exists
+-- Test with parent and ns attribute --
+DOMNameSpaceNode Object
+(
+    [nodeName] => xmlns:xsi
+    [nodeValue] => fooooooooooooooooooooo
+    [nodeType] => 18
+    [prefix] => xsi
+    [localName] => xsi
+    [namespaceURI] => fooooooooooooooooooooo
+    [ownerDocument] => (object value omitted)
+    [parentNode] => (object value omitted)
+)
+string(3) "url"

ext/dom/tests/bug78577.phpt

@@ -0,0 +1,33 @@
+--TEST--
+Bug #78577 (Crash in DOMNameSpace debug info handlers)
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$doc = new DOMDocument;
+$doc->loadXML('<foo xmlns="http://php.net/test" xmlns:foo="urn:foo" />');
+
+$attr = $doc->documentElement->getAttributeNode('xmlns');
+var_dump($attr);
+
+?>
+--EXPECT--
+object(DOMNameSpaceNode)#3 (8) {
+  ["nodeName"]=>
+  string(5) "xmlns"
+  ["nodeValue"]=>
+  string(19) "http://php.net/test"
+  ["nodeType"]=>
+  int(18)
+  ["prefix"]=>
+  string(0) ""
+  ["localName"]=>
+  string(5) "xmlns"
+  ["namespaceURI"]=>
+  string(19) "http://php.net/test"
+  ["ownerDocument"]=>
+  string(22) "(object value omitted)"
+  ["parentNode"]=>
+  string(22) "(object value omitted)"
+}

ext/dom/tests/xpath_domnamespacenode.phpt

@@ -17,7 +17,7 @@ var_dump($nodes->item(0));
 
 ?>
 --EXPECT--
-object(DOMNameSpaceNode)#3 (8) {
+object(DOMNameSpaceNode)#4 (8) {
   ["nodeName"]=>
   string(9) "xmlns:xml"
   ["nodeValue"]=>

ext/dom/tests/xpath_domnamespacenode_advanced.phpt

@@ -0,0 +1,75 @@
+--TEST--
+DOMXPath::query() can return DOMNodeList with DOMNameSpaceNode items - advanced variation
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = new DOMDocument();
+$dom->loadXML(<<<'XML'
+<root xmlns:foo="http://example.com/foo" xmlns:bar="http://example.com/bar">
+    <child xmlns:baz="http://example.com/baz">Hello PHP!</child>
+</root>
+XML);
+
+$xpath = new DOMXPath($dom);
+$query = '//namespace::*';
+
+echo "-- All namespace attributes --\n";
+
+foreach ($xpath->query($query) as $attribute) {
+    echo $attribute->nodeName . ' = ' . $attribute->nodeValue . PHP_EOL;
+    var_dump($attribute->parentNode->tagName);
+}
+
+echo "-- All namespace attributes with removal attempt --\n";
+
+foreach ($xpath->query($query) as $attribute) {
+    echo "Before: ", $attribute->parentNode->tagName, "\n";
+    // Second & third attempt should fail because it's no longer in the document
+    try {
+        $attribute->parentNode->remove();
+    } catch (\DOMException $e) {
+        echo $e->getMessage(), "\n";
+    }
+    // However, it should not cause a use-after-free
+    echo "After: ", $attribute->parentNode->tagName, "\n";
+}
+
+?>
+--EXPECT--
+-- All namespace attributes --
+xmlns:xml = http://www.w3.org/XML/1998/namespace
+string(4) "root"
+xmlns:bar = http://example.com/bar
+string(4) "root"
+xmlns:foo = http://example.com/foo
+string(4) "root"
+xmlns:xml = http://www.w3.org/XML/1998/namespace
+string(5) "child"
+xmlns:bar = http://example.com/bar
+string(5) "child"
+xmlns:foo = http://example.com/foo
+string(5) "child"
+xmlns:baz = http://example.com/baz
+string(5) "child"
+-- All namespace attributes with removal attempt --
+Before: root
+After: root
+Before: root
+Not Found Error
+After: root
+Before: root
+Not Found Error
+After: root
+Before: child
+After: child
+Before: child
+Not Found Error
+After: child
+Before: child
+Not Found Error
+After: child
+Before: child
+Not Found Error
+After: child