Skip to content

Commit e878b9f

Browse files
ndosschendossche
committed
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.
1 parent d670e13 commit e878b9f

File tree

8 files changed

+175
-3
lines changed

8 files changed

+175
-3
lines changed

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ PHP NEWS
22
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
33
?? ??? ????, PHP 8.2.20
44

5+
- DOM:
6+
. Fix crashes when entity declaration is removed while still having entity
7+
references. (nielsdos)
58

69
09 May 2024, PHP 8.2.19
710

‎ext/dom/dom_properties.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ int dom_entity_actual_encoding_read(dom_object *obj, zval *retval);
8181
int dom_entity_encoding_read(dom_object *obj, zval *retval);
8282
int dom_entity_version_read(dom_object *obj, zval *retval);
8383

84+
/* entity reference properties */
85+
int dom_entity_reference_child_read(dom_object *obj, zval *retval);
86+
int dom_entity_reference_text_content_read(dom_object *obj, zval *retval);
87+
int dom_entity_reference_child_nodes_read(dom_object *obj, zval *retval);
88+
8489
/* namednodemap properties */
8590
int dom_namednodemap_length_read(dom_object *obj, zval *retval);
8691

‎ext/dom/entityreference.c‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "php.h"
2323
#if defined(HAVE_LIBXML) && defined(HAVE_DOM)
2424
#include "php_dom.h"
25+
#include "dom_properties.h"
2526

2627
/*
2728
* class DOMEntityReference extends DOMNode
@@ -65,4 +66,63 @@ PHP_METHOD(DOMEntityReference, __construct)
6566
}
6667
/* }}} end DOMEntityReference::__construct */
6768

69+
/* The following property handlers are necessary because of special lifetime management with entities and entity
70+
* references. The issue is that entity references hold a reference to an entity declaration, but don't
71+
* register that reference anywhere. When the entity declaration disappears we have no way of notifying the
72+
* entity references. Override the property handlers for the declaration-accessing properties to fix this problem. */
73+
74+
xmlEntityPtr dom_entity_reference_fetch_and_sync_declaration(xmlNodePtr reference)
75+
{
76+
xmlEntityPtr entity = xmlGetDocEntity(reference->doc, reference->name);
77+
reference->children = (xmlNodePtr) entity;
78+
reference->last = (xmlNodePtr) entity;
79+
reference->content = entity ? entity->content : NULL;
80+
return entity;
81+
}
82+
83+
int dom_entity_reference_child_read(dom_object *obj, zval *retval)
84+
{
85+
xmlNodePtr nodep = dom_object_get_node(obj);
86+
87+
if (nodep == NULL) {
88+
php_dom_throw_error(INVALID_STATE_ERR, true);
89+
return FAILURE;
90+
}
91+
92+
xmlEntityPtr entity = dom_entity_reference_fetch_and_sync_declaration(nodep);
93+
if (entity == NULL) {
94+
ZVAL_NULL(retval);
95+
return SUCCESS;
96+
}
97+
98+
php_dom_create_object((xmlNodePtr) entity, retval, obj);
99+
return SUCCESS;
100+
}
101+
102+
int dom_entity_reference_text_content_read(dom_object *obj, zval *retval)
103+
{
104+
xmlNodePtr nodep = dom_object_get_node(obj);
105+
106+
if (nodep == NULL) {
107+
php_dom_throw_error(INVALID_STATE_ERR, true);
108+
return FAILURE;
109+
}
110+
111+
dom_entity_reference_fetch_and_sync_declaration(nodep);
112+
return dom_node_text_content_read(obj, retval);
113+
}
114+
115+
int dom_entity_reference_child_nodes_read(dom_object *obj, zval *retval)
116+
{
117+
xmlNodePtr nodep = dom_object_get_node(obj);
118+
119+
if (nodep == NULL) {
120+
php_dom_throw_error(INVALID_STATE_ERR, true);
121+
return FAILURE;
122+
}
123+
124+
dom_entity_reference_fetch_and_sync_declaration(nodep);
125+
return dom_node_child_nodes_read(obj, retval);
126+
}
127+
68128
#endif

‎ext/dom/nodelist.c‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@
3131
* Since:
3232
*/
3333

34+
static xmlNodePtr dom_nodelist_iter_start_first_child(xmlNodePtr nodep)
35+
{
36+
if (nodep->type == XML_ENTITY_REF_NODE) {
37+
/* See entityreference.c */
38+
dom_entity_reference_fetch_and_sync_declaration(nodep);
39+
}
40+
41+
return nodep->children;
42+
}
43+
3444
int php_dom_get_nodelist_length(dom_object *obj)
3545
{
3646
dom_nnodemap_object *objmap = (dom_nnodemap_object *) obj->ptr;
@@ -54,7 +64,7 @@ int php_dom_get_nodelist_length(dom_object *obj)
5464

5565
int count = 0;
5666
if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
57-
xmlNodePtr curnode = nodep->children;
67+
xmlNodePtr curnode = dom_nodelist_iter_start_first_child(nodep);
5868
if (curnode) {
5969
count++;
6070
while (curnode->next != NULL) {
@@ -128,7 +138,7 @@ void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long
128138
if (nodep) {
129139
int count = 0;
130140
if (objmap->nodetype == XML_ATTRIBUTE_NODE || objmap->nodetype == XML_ELEMENT_NODE) {
131-
xmlNodePtr curnode = nodep->children;
141+
xmlNodePtr curnode = dom_nodelist_iter_start_first_child(nodep);
132142
while (count < index && curnode != NULL) {
133143
count++;
134144
curnode = curnode->next;

‎ext/dom/php_dom.c‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ static HashTable classes;
7272
static HashTable dom_document_prop_handlers;
7373
static HashTable dom_documentfragment_prop_handlers;
7474
static HashTable dom_node_prop_handlers;
75+
static HashTable dom_entity_reference_prop_handlers;
7576
static HashTable dom_nodelist_prop_handlers;
7677
static HashTable dom_namednodemap_prop_handlers;
7778
static HashTable dom_characterdata_prop_handlers;
@@ -284,6 +285,14 @@ static void dom_register_prop_handler(HashTable *prop_handler, char *name, size_
284285
zend_string_release_ex(str, 1);
285286
}
286287

288+
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)
289+
{
290+
dom_prop_handler hnd;
291+
hnd.read_func = read_func;
292+
hnd.write_func = write_func;
293+
zend_hash_str_update_mem(prop_handler, name, name_len, &hnd, sizeof(dom_prop_handler));
294+
}
295+
287296
static zval *dom_get_property_ptr_ptr(zend_object *object, zend_string *name, int type, void **cache_slot)
288297
{
289298
dom_object *obj = php_dom_obj_from_obj(object);
@@ -807,7 +816,14 @@ PHP_MINIT_FUNCTION(dom)
807816

808817
dom_entityreference_class_entry = register_class_DOMEntityReference(dom_node_class_entry);
809818
dom_entityreference_class_entry->create_object = dom_objects_new;
810-
zend_hash_add_ptr(&classes, dom_entityreference_class_entry->name, &dom_node_prop_handlers);
819+
820+
zend_hash_init(&dom_entity_reference_prop_handlers, 0, NULL, dom_dtor_prop_handler, true);
821+
zend_hash_merge(&dom_entity_reference_prop_handlers, &dom_node_prop_handlers, dom_copy_prop_handler, false);
822+
dom_override_prop_handler(&dom_entity_reference_prop_handlers, "firstChild", sizeof("firstChild")-1, dom_entity_reference_child_read, NULL);
823+
dom_override_prop_handler(&dom_entity_reference_prop_handlers, "lastChild", sizeof("lastChild")-1, dom_entity_reference_child_read, NULL);
824+
dom_override_prop_handler(&dom_entity_reference_prop_handlers, "textContent", sizeof("textContent")-1, dom_entity_reference_text_content_read, NULL);
825+
dom_override_prop_handler(&dom_entity_reference_prop_handlers, "childNodes", sizeof("childNodes")-1, dom_entity_reference_child_nodes_read, NULL);
826+
zend_hash_add_ptr(&classes, dom_entityreference_class_entry->name, &dom_entity_reference_prop_handlers);
811827

812828
dom_processinginstruction_class_entry = register_class_DOMProcessingInstruction(dom_node_class_entry);
813829
dom_processinginstruction_class_entry->create_object = dom_objects_new;
@@ -869,6 +885,7 @@ PHP_MSHUTDOWN_FUNCTION(dom) /* {{{ */
869885
zend_hash_destroy(&dom_document_prop_handlers);
870886
zend_hash_destroy(&dom_documentfragment_prop_handlers);
871887
zend_hash_destroy(&dom_node_prop_handlers);
888+
zend_hash_destroy(&dom_entity_reference_prop_handlers);
872889
zend_hash_destroy(&dom_namespace_node_prop_handlers);
873890
zend_hash_destroy(&dom_nodelist_prop_handlers);
874891
zend_hash_destroy(&dom_namednodemap_prop_handlers);

‎ext/dom/php_dom.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ xmlNode *php_dom_libxml_notation_iter(xmlHashTable *ht, int index);
139139
zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, int by_ref);
140140
void dom_set_doc_classmap(php_libxml_ref_obj *document, zend_class_entry *basece, zend_class_entry *ce);
141141
xmlNodePtr php_dom_create_fake_namespace_decl(xmlNodePtr nodep, xmlNsPtr original, zval *return_value, dom_object *parent_intern);
142+
xmlEntityPtr dom_entity_reference_fetch_and_sync_declaration(xmlNodePtr reference);
142143

143144
void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc);
144145
void dom_parent_node_append(dom_object *context, zval *nodes, int nodesc);

‎ext/dom/tests/entity_reference_stale_01.phpt‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
Entity references with stale entity declaration 01
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = new DOMDocument;
9+
$dom->loadXML(<<<XML
10+
<!DOCTYPE foo [
11+
<!ENTITY foo "bar">
12+
]>
13+
<foo>&foo;</foo>
14+
XML);
15+
16+
$ref = $dom->documentElement->firstChild;
17+
$decl = $ref->firstChild;
18+
19+
$nodes = $ref->childNodes;
20+
$dom->removeChild($dom->doctype);
21+
unset($decl);
22+
23+
var_dump($nodes);
24+
var_dump($ref->firstChild);
25+
var_dump($ref->lastChild);
26+
var_dump($ref->textContent);
27+
var_dump($ref->childNodes);
28+
29+
?>
30+
--EXPECT--
31+
object(DOMNodeList)#4 (1) {
32+
["length"]=>
33+
int(0)
34+
}
35+
NULL
36+
NULL
37+
string(0) ""
38+
object(DOMNodeList)#2 (1) {
39+
["length"]=>
40+
int(0)
41+
}

‎ext/dom/tests/entity_reference_stale_02.phpt‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
Entity references with stale entity declaration 02
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
$dom = new DOMDocument;
9+
$dom->loadXML(<<<XML
10+
<!DOCTYPE foo [
11+
<!ENTITY foo1 "bar1">
12+
<!ENTITY foo2 "bar2">
13+
<!ENTITY foo3 "bar3">
14+
]>
15+
<foo>&foo1;</foo>
16+
XML);
17+
18+
$ref = $dom->documentElement->firstChild;
19+
$decl = $ref->firstChild;
20+
21+
$nodes = $ref->childNodes;
22+
$iter = $nodes->getIterator();
23+
$iter->next();
24+
$dom->removeChild($dom->doctype);
25+
unset($decl);
26+
27+
try {
28+
$iter->current()->publicId;
29+
} catch (Error $e) {
30+
echo $e->getMessage(), "\n";
31+
}
32+
33+
?>
34+
--EXPECT--
35+
Couldn't fetch DOMEntity. Node no longer exists

0 commit comments

Comments
 (0)