From 100d5b2768aa108146d0aa77aaee4f0f3366031a Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 4 Dec 2024 22:51:12 +0100 Subject: [PATCH] Fix GH-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. --- ext/simplexml/simplexml.c | 25 +++++++++++++++---------- ext/simplexml/tests/gh17040.phpt | 24 ++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 10 deletions(-) create mode 100644 ext/simplexml/tests/gh17040.phpt diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 6a5a514f9a3d3..6f4a0d6316ec8 100644 --- a/ext/simplexml/simplexml.c +++ b/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: diff --git a/ext/simplexml/tests/gh17040.phpt b/ext/simplexml/tests/gh17040.phpt new file mode 100644 index 0000000000000..20dc9062d559b --- /dev/null +++ b/ext/simplexml/tests/gh17040.phpt @@ -0,0 +1,24 @@ +--TEST-- +GH-17040 (SimpleXML's unset can break DOM objects) +--EXTENSIONS-- +dom +simplexml +--FILE-- +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-- + + + +object(SimpleXMLElement)#3 (1) { + [0]=> + string(3) "foo" +}