diff --git a/NEWS b/NEWS index facc4a1d0031..a57dfc54b621 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,8 @@ PHP NEWS - DOM: . Fixed bug GH-25268 (NULL php_libxml_ref_obj dereference in dom_objects_store_clone_obj when cloning Dom\Notation). (David Carlier) + . Fixed use-after-free in getElementById() after a removed ID attribute is + freed. (Ilia Alshanetsky) - MBString: . Fixed stack overflow in mb_check_encoding() and mb_convert_encoding() with diff --git a/ext/dom/tests/getElementById_removed_id_attribute.phpt b/ext/dom/tests/getElementById_removed_id_attribute.phpt new file mode 100644 index 000000000000..56930a0b06e9 --- /dev/null +++ b/ext/dom/tests/getElementById_removed_id_attribute.phpt @@ -0,0 +1,56 @@ +--TEST-- +getElementById() after a removed ID attribute is freed +--EXTENSIONS-- +dom +--FILE-- +loadXML(''); +$a = $doc->documentElement->firstChild; +$attr = $a->getAttributeNode('xml:id'); +$a->removeAttributeNode($attr); +unset($attr); +echo "removed id: "; +var_dump($doc->getElementById('x')); +echo "remaining id: ", $doc->getElementById('y')->nodeName, "\n"; + +$doc = new DOMDocument(); +$doc->loadXML(']>'); +$e = $doc->documentElement->firstChild; +$attr = $e->getAttributeNode('id'); +$e->removeAttributeNode($attr); +unset($attr); +echo "removed DTD id: "; +var_dump($doc->getElementById('x')); +echo "remaining DTD id: ", $doc->getElementById('y')->nodeName, "\n"; + +$doc = Dom\XMLDocument::createFromString(''); +$a = $doc->documentElement->firstChild; +$attr = $a->getAttributeNode('xml:id'); +$a->removeAttributeNode($attr); +unset($attr); +echo "removed Dom\\XMLDocument id: "; +var_dump($doc->getElementById('x')); +echo "remaining Dom\\XMLDocument id: ", $doc->getElementById('y')->nodeName, "\n"; + +$doc = new DOMDocument(); +$doc->loadXML(''); +$a = $doc->documentElement->firstChild; +$attr = $a->getAttributeNode('xml:id'); +$a->removeAttribute('xml:id'); +unset($attr); +echo "removeAttribute() id: "; +var_dump($doc->getElementById('x')); +echo "remaining removeAttribute() id: ", $doc->getElementById('y')->nodeName, "\n"; + +?> +--EXPECT-- +removed id: NULL +remaining id: b +removed DTD id: NULL +remaining DTD id: e +removed Dom\XMLDocument id: NULL +remaining Dom\XMLDocument id: b +removeAttribute() id: NULL +remaining removeAttribute() id: b diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c index cb8a1df0efdd..7665de60038e 100644 --- a/ext/libxml/libxml.c +++ b/ext/libxml/libxml.c @@ -1480,6 +1480,9 @@ PHP_LIBXML_API void php_libxml_node_free_resource(xmlNodePtr node) break; default: if (node->parent == NULL || node->type == XML_NAMESPACE_DECL) { + if (node->type == XML_ATTRIBUTE_NODE && node->doc != NULL && ((xmlAttrPtr) node)->atype == XML_ATTRIBUTE_ID) { + xmlRemoveID(node->doc, (xmlAttrPtr) node); + } php_libxml_node_free_list((xmlNodePtr) node->children); if (node->type == XML_ELEMENT_NODE) { php_libxml_node_free_list((xmlNodePtr) node->properties);