Fix phpGH-17040: SimpleXML's unset can break DOM objects

Don't free the underlying nodes if we still have objects pointing to
them, otherwise the objects are left with a NULL node pointer.

Closes phpGH-17046.

Author: ndossche

NEWS

@@ -8,6 +8,9 @@ PHP                                                                        NEWS
 - Iconv:
   . Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos)
 
+- SimpleXML:
+  . Fixed bug GH-17040 (SimpleXML's unset can break DOM objects). (nielsdos)
+
 - Streams:
   . Fixed bug GH-17037 (UAF in user filter when adding existing filter name due
     to incorrect error handling). (nielsdos)

ext/simplexml/simplexml.c

@@ -54,6 +54,16 @@ static void php_sxe_iterator_move_forward(zend_object_iterator *iter);
 static void php_sxe_iterator_rewind(zend_object_iterator *iter);
 static zend_result sxe_object_cast_ex(zend_object *readobj, zval *writeobj, int type);
 
+static void sxe_unlink_node(xmlNodePtr node)
+{
+	xmlUnlinkNode(node);
+	/* Only destroy the nodes if we have no objects using them anymore.
+	 * Don't assume simplexml owns these. */
+	if (!node->_private) {
+		php_libxml_node_free_resource(node);
+	}
+}
+
 /* {{{ _node_as_zval() */
 static void _node_as_zval(php_sxe_object *sxe, xmlNodePtr node, zval *value, SXE_ITER itertype, char *name, const xmlChar *nsprefix, int isprefix)
 {
@@ -554,8 +564,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
 			}
 			if (value_str) {
 				while ((tempnode = (xmlNodePtr) newnode->children)) {
-					xmlUnlinkNode(tempnode);
-					php_libxml_node_free_resource((xmlNodePtr) tempnode);
+					sxe_unlink_node(tempnode);
 				}
 				change_node_zval(newnode, value_str);
 			}
@@ -829,8 +838,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
 				while (attr && nodendx <= Z_LVAL_P(member)) {
 					if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
 						if (nodendx == Z_LVAL_P(member)) {
-							xmlUnlinkNode((xmlNodePtr) attr);
-							php_libxml_node_free_resource((xmlNodePtr) attr);
+							sxe_unlink_node((xmlNodePtr) attr);
 							break;
 						}
 						nodendx++;
@@ -841,8 +849,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
 				while (attr) {
 					anext = attr->next;
 					if ((!test || xmlStrEqual(attr->name, sxe->iter.name)) && xmlStrEqual(attr->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, (xmlNodePtr) attr, sxe->iter.nsprefix, sxe->iter.isprefix)) {
-						xmlUnlinkNode((xmlNodePtr) attr);
-						php_libxml_node_free_resource((xmlNodePtr) attr);
+						sxe_unlink_node((xmlNodePtr) attr);
 						break;
 					}
 					attr = anext;
@@ -857,8 +864,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
 				}
 				node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, NULL);
 				if (node) {
-					xmlUnlinkNode(node);
-					php_libxml_node_free_resource(node);
+					sxe_unlink_node(node);
 				}
 			} else {
 				node = node->children;
@@ -868,8 +874,7 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements
 					SKIP_TEXT(node);
 
 					if (xmlStrEqual(node->name, (xmlChar *)Z_STRVAL_P(member)) && match_ns(sxe, node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
-						xmlUnlinkNode(node);
-						php_libxml_node_free_resource(node);
+						sxe_unlink_node(node);
 					}
 
 next_iter:

ext/simplexml/tests/gh17040.phpt

@@ -0,0 +1,24 @@
+--TEST--
+GH-17040 (SimpleXML's unset can break DOM objects)
+--EXTENSIONS--
+dom
+simplexml
+--FILE--
+<?php
+$dom = new DOMDocument;
+$tag = $dom->appendChild($dom->createElement("style"));
+$html = simplexml_import_dom($tag);
+unset($html[0]);
+$tag->append("foo");
+echo $dom->saveXML(), "\n";
+echo $dom->saveXML($tag), "\n";
+var_dump($html);
+?>
+--EXPECT--
+<?xml version="1.0"?>
+
+<style>foo</style>
+object(SimpleXMLElement)#3 (1) {
+  [0]=>
+  string(3) "foo"
+}