Fix crashes when entity declaration is removed while still having entity references

libxml doesn't do reference counting inside its node types. It's
possible to remove an entity declaration out of the document, but then
entity references will keep pointing to that stale declaration. This
will cause crashes.

One idea would be to check when a declaration is removed, to trigger a
hook that updates all references. However this means we have to keep
track of all references somehow, which would be a high-overhead
solution. The solution in this patch makes sure that the fields are
always updated before they are read.

Closes GH-14089.

NEWS

@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? ????, PHP 8.2.20
 
+- DOM:
+  . Fix crashes when entity declaration is removed while still having entity
+    references. (nielsdos)
 
 09 May 2024, PHP 8.2.19
 

ext/dom/dom_properties.h

@@ -81,6 +81,11 @@ int dom_entity_actual_encoding_read(dom_object *obj, zval *retval);
 int dom_entity_encoding_read(dom_object *obj, zval *retval);
 int dom_entity_version_read(dom_object *obj, zval *retval);
 
+/* entity reference properties */
+int dom_entity_reference_child_read(dom_object *obj, zval *retval);
+int dom_entity_reference_text_content_read(dom_object *obj, zval *retval);
+int dom_entity_reference_child_nodes_read(dom_object *obj, zval *retval);
+
 /* namednodemap properties */
 int dom_namednodemap_length_read(dom_object *obj, zval *retval);
 

ext/dom/entityreference.c

@@ -22,6 +22,7 @@
 #include "php.h"
 #if defined(HAVE_LIBXML) && defined(HAVE_DOM)
 #include "php_dom.h"
+#include "dom_properties.h"
 
 /*
 * class DOMEntityReference extends DOMNode
@@ -65,4 +66,63 @@ PHP_METHOD(DOMEntityReference, __construct)
 }
 /* }}} end DOMEntityReference::__construct */
 
+/* The following property handlers are necessary because of special lifetime management with entities and entity
+ * references. The issue is that entity references hold a reference to an entity declaration, but don't
+ * register that reference anywhere. When the entity declaration disappears we have no way of notifying the
+ * entity references. Override the property handlers for the declaration-accessing properties to fix this problem. */
+
+xmlEntityPtr dom_entity_reference_fetch_and_sync_declaration(xmlNodePtr reference)
+{
+	xmlEntityPtr entity = xmlGetDocEntity(reference->doc, reference->name);
+	reference->children = (xmlNodePtr) entity;
+	reference->last = (xmlNodePtr) entity;
+	reference->content = entity ? entity->content : NULL;
+	return entity;
+}
+
+int dom_entity_reference_child_read(dom_object *obj, zval *retval)
+{
+	xmlNodePtr nodep = dom_object_get_node(obj);
+
+	if (nodep == NULL) {
+		php_dom_throw_error(INVALID_STATE_ERR, true);
+		return FAILURE;
+	}
+
+	xmlEntityPtr entity = dom_entity_reference_fetch_and_sync_declaration(nodep);
+	if (entity == NULL) {
+		ZVAL_NULL(retval);
+		return SUCCESS;
+	}
+
+	php_dom_create_object((xmlNodePtr) entity, retval, obj);
+	return SUCCESS;
+}
+
+int dom_entity_reference_text_content_read(dom_object *obj, zval *retval)
+{
+	xmlNodePtr nodep = dom_object_get_node(obj);
+
+	if (nodep == NULL) {
+		php_dom_throw_error(INVALID_STATE_ERR, true);
+		return FAILURE;
+	}
+
+	dom_entity_reference_fetch_and_sync_declaration(nodep);
+	return dom_node_text_content_read(obj, retval);
+}
+
+int dom_entity_reference_child_nodes_read(dom_object *obj, zval *retval)
+{
+	xmlNodePtr nodep = dom_object_get_node(obj);
+
+	if (nodep == NULL) {
+		php_dom_throw_error(INVALID_STATE_ERR, true);
+		return FAILURE;
+	}
+
+	dom_entity_reference_fetch_and_sync_declaration(nodep);
+	return dom_node_child_nodes_read(obj, retval);
+}
+
 #endif

ext/dom/nodelist.c

@@ -31,6 +31,16 @@
 * Since:
 */
 
+static xmlNodePtr dom_nodelist_iter_start_first_child(xmlNodePtr nodep)
+{
+	if (nodep->type == XML_ENTITY_REF_NODE) {
+		/* See entityreference.c */
+		dom_entity_reference_fetch_and_sync_declaration(nodep);
+	}
+
+	return nodep->children;
+}
+
 int php_dom_get_nodelist_length(dom_object *obj)
 {
 	dom_nnodemap_object *objmap = (dom_nnodemap_object *) obj->ptr;
@@ -54,7 +64,7 @@ int php_dom_get_nodelist_length(dom_object *obj)
 
 	int count = 0;
 	if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
-		xmlNodePtr curnode = nodep->children;
+		xmlNodePtr curnode = dom_nodelist_iter_start_first_child(nodep);
 		if (curnode) {
 			count++;
 			while (curnode->next != NULL) {
@@ -128,7 +138,7 @@ void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long
 					if (nodep) {
 						int count = 0;
 						if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
-							xmlNodePtr curnode = nodep->children;
+							xmlNodePtr curnode = dom_nodelist_iter_start_first_child(nodep);
 							while (count < index && curnode != NULL) {
 								count++;
 								curnode = curnode->next;

ext/dom/php_dom.c

@@ -72,6 +72,7 @@ static HashTable classes;
 static HashTable dom_document_prop_handlers;
 static HashTable dom_documentfragment_prop_handlers;
 static HashTable dom_node_prop_handlers;
+static HashTable dom_entity_reference_prop_handlers;
 static HashTable dom_nodelist_prop_handlers;
 static HashTable dom_namednodemap_prop_handlers;
 static HashTable dom_characterdata_prop_handlers;
@@ -284,6 +285,14 @@ static void dom_register_prop_handler(HashTable *prop_handler, char *name, size_
 	zend_string_release_ex(str, 1);
 }
 
+static void dom_override_prop_handler(HashTable *prop_handler, char *name, size_t name_len, dom_read_t read_func, dom_write_t write_func)
+{
+	dom_prop_handler hnd;
+	hnd.read_func = read_func;
+	hnd.write_func = write_func;
+	zend_hash_str_update_mem(prop_handler, name, name_len, &hnd, sizeof(dom_prop_handler));
+}
+
 static zval *dom_get_property_ptr_ptr(zend_object *object, zend_string *name, int type, void **cache_slot)
 {
 	dom_object *obj = php_dom_obj_from_obj(object);
@@ -807,7 +816,14 @@ PHP_MINIT_FUNCTION(dom)
 
 	dom_entityreference_class_entry = register_class_DOMEntityReference(dom_node_class_entry);
 	dom_entityreference_class_entry->create_object = dom_objects_new;
-	zend_hash_add_ptr(&classes, dom_entityreference_class_entry->name, &dom_node_prop_handlers);
+
+	zend_hash_init(&dom_entity_reference_prop_handlers, 0, NULL, dom_dtor_prop_handler, true);
+	zend_hash_merge(&dom_entity_reference_prop_handlers, &dom_node_prop_handlers, dom_copy_prop_handler, false);
+	dom_override_prop_handler(&dom_entity_reference_prop_handlers, "firstChild", sizeof("firstChild")-1, dom_entity_reference_child_read, NULL);
+	dom_override_prop_handler(&dom_entity_reference_prop_handlers, "lastChild", sizeof("lastChild")-1, dom_entity_reference_child_read, NULL);
+	dom_override_prop_handler(&dom_entity_reference_prop_handlers, "textContent", sizeof("textContent")-1, dom_entity_reference_text_content_read, NULL);
+	dom_override_prop_handler(&dom_entity_reference_prop_handlers, "childNodes", sizeof("childNodes")-1, dom_entity_reference_child_nodes_read, NULL);
+	zend_hash_add_ptr(&classes, dom_entityreference_class_entry->name, &dom_entity_reference_prop_handlers);
 
 	dom_processinginstruction_class_entry = register_class_DOMProcessingInstruction(dom_node_class_entry);
 	dom_processinginstruction_class_entry->create_object = dom_objects_new;
@@ -869,6 +885,7 @@ PHP_MSHUTDOWN_FUNCTION(dom) /* {{{ */
 	zend_hash_destroy(&dom_document_prop_handlers);
 	zend_hash_destroy(&dom_documentfragment_prop_handlers);
 	zend_hash_destroy(&dom_node_prop_handlers);
+	zend_hash_destroy(&dom_entity_reference_prop_handlers);
 	zend_hash_destroy(&dom_namespace_node_prop_handlers);
 	zend_hash_destroy(&dom_nodelist_prop_handlers);
 	zend_hash_destroy(&dom_namednodemap_prop_handlers);

ext/dom/php_dom.h

@@ -139,6 +139,7 @@ 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);
+xmlEntityPtr dom_entity_reference_fetch_and_sync_declaration(xmlNodePtr reference);
 
 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/entity_reference_stale_01.phpt

@@ -0,0 +1,41 @@
+--TEST--
+Entity references with stale entity declaration 01
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = new DOMDocument;
+$dom->loadXML(<<<XML
+<!DOCTYPE foo [
+<!ENTITY foo "bar">
+]>
+<foo>&foo;</foo>
+XML);
+
+$ref = $dom->documentElement->firstChild;
+$decl = $ref->firstChild;
+
+$nodes = $ref->childNodes;
+$dom->removeChild($dom->doctype);
+unset($decl);
+
+var_dump($nodes);
+var_dump($ref->firstChild);
+var_dump($ref->lastChild);
+var_dump($ref->textContent);
+var_dump($ref->childNodes);
+
+?>
+--EXPECT--
+object(DOMNodeList)#4 (1) {
+  ["length"]=>
+  int(0)
+}
+NULL
+NULL
+string(0) ""
+object(DOMNodeList)#2 (1) {
+  ["length"]=>
+  int(0)
+}

ext/dom/tests/entity_reference_stale_02.phpt

@@ -0,0 +1,35 @@
+--TEST--
+Entity references with stale entity declaration 02
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+
+$dom = new DOMDocument;
+$dom->loadXML(<<<XML
+<!DOCTYPE foo [
+<!ENTITY foo1 "bar1">
+<!ENTITY foo2 "bar2">
+<!ENTITY foo3 "bar3">
+]>
+<foo>&foo1;</foo>
+XML);
+
+$ref = $dom->documentElement->firstChild;
+$decl = $ref->firstChild;
+
+$nodes = $ref->childNodes;
+$iter = $nodes->getIterator();
+$iter->next();
+$dom->removeChild($dom->doctype);
+unset($decl);
+
+try {
+    $iter->current()->publicId;
+} catch (Error $e) {
+    echo $e->getMessage(), "\n";
+}
+
+?>
+--EXPECT--
+Couldn't fetch DOMEntity. Node no longer exists