Skip to content

Commit 8a6de2c

Browse files
ndosschendossche
committed
Merge branch 'PHP-8.4'
* PHP-8.4: Fix bugs phpGH-16150 and phpGH-16152: intern document mismanagement
2 parents 807a74a + ba0834b commit 8a6de2c

File tree

3 files changed

+97
-30
lines changed

3 files changed

+97
-30
lines changed

‎ext/dom/node.c‎

Lines changed: 43 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -740,11 +740,43 @@ zend_result dom_node_text_content_write(dom_object *obj, zval *newval)
740740

741741
/* }}} */
742742

743-
static xmlNodePtr dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlNodePtr nextsib, xmlNodePtr fragment, dom_object *intern) /* {{{ */
743+
/* Returns true if the node was changed, false otherwise. */
744+
static bool dom_set_document_ref_obj_single(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
744745
{
745-
xmlNodePtr newchild, node;
746+
dom_object *childobj = php_dom_object_get_data(node);
747+
if (childobj && !childobj->document) {
748+
childobj->document = document;
749+
document->refcount++;
750+
return true;
751+
}
752+
return false;
753+
}
754+
755+
static void dom_set_document_pointers(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
756+
{
757+
/* Applies the document to the entire subtree. */
758+
xmlSetTreeDoc(node, doc);
759+
760+
if (!dom_set_document_ref_obj_single(node, doc, document)) {
761+
return;
762+
}
763+
764+
xmlNodePtr base = node;
765+
node = node->children;
766+
while (node != NULL) {
767+
ZEND_ASSERT(node != base);
768+
769+
if (!dom_set_document_ref_obj_single(node, doc, document)) {
770+
break;
771+
}
746772

747-
newchild = fragment->children;
773+
node = php_dom_next_in_tree_order(node, base);
774+
}
775+
}
776+
777+
static xmlNodePtr dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlNodePtr nextsib, xmlNodePtr fragment, dom_object *intern) /* {{{ */
778+
{
779+
xmlNodePtr newchild = fragment->children;
748780

749781
if (newchild) {
750782
if (prevsib == NULL) {
@@ -760,17 +792,10 @@ static xmlNodePtr dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlN
760792
nextsib->prev = fragment->last;
761793
}
762794

763-
node = newchild;
795+
/* Assign parent node pointer */
796+
xmlNodePtr node = newchild;
764797
while (node != NULL) {
765798
node->parent = nodep;
766-
if (node->doc != nodep->doc) {
767-
xmlSetTreeDoc(node, nodep->doc);
768-
dom_object *childobj = node->_private;
769-
if (childobj != NULL) {
770-
childobj->document = intern->document;
771-
php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
772-
}
773-
}
774799
if (node == fragment->last) {
775800
break;
776801
}
@@ -835,8 +860,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
835860
}
836861

837862
if (child->doc == NULL && parentp->doc != NULL) {
838-
childobj->document = intern->document;
839-
php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
863+
dom_set_document_pointers(child, parentp->doc, intern->document);
840864
}
841865

842866
php_libxml_invalidate_node_list_cache(intern->document);
@@ -856,9 +880,6 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
856880

857881
if (child->type == XML_TEXT_NODE && (refp->type == XML_TEXT_NODE ||
858882
(refp->prev != NULL && refp->prev->type == XML_TEXT_NODE))) {
859-
if (child->doc == NULL) {
860-
xmlSetTreeDoc(child, parentp->doc);
861-
}
862883
new_child = child;
863884
new_child->parent = refp->parent;
864885
new_child->next = refp;
@@ -910,9 +931,6 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj
910931
}
911932
if (child->type == XML_TEXT_NODE && parentp->last != NULL && parentp->last->type == XML_TEXT_NODE) {
912933
child->parent = parentp;
913-
if (child->doc == NULL) {
914-
xmlSetTreeDoc(child, parentp->doc);
915-
}
916934
new_child = child;
917935
if (parentp->children == NULL) {
918936
parentp->children = child;
@@ -1155,6 +1173,10 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern)
11551173
}
11561174
}
11571175

1176+
if (newchild->doc == NULL && nodep->doc != NULL) {
1177+
dom_set_document_pointers(newchild, nodep->doc, intern->document);
1178+
}
1179+
11581180
if (newchild->type == XML_DOCUMENT_FRAG_NODE) {
11591181
xmlNodePtr prevsib, nextsib;
11601182
prevsib = oldchild->prev;
@@ -1171,11 +1193,6 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern)
11711193
xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc);
11721194
bool replacedoctype = (intSubset == (xmlDtd *) oldchild);
11731195

1174-
if (newchild->doc == NULL && nodep->doc != NULL) {
1175-
xmlSetTreeDoc(newchild, nodep->doc);
1176-
newchildobj->document = intern->document;
1177-
php_libxml_increment_doc_ref((php_libxml_node_object *)newchildobj, NULL);
1178-
}
11791196
xmlReplaceNode(oldchild, newchild);
11801197
if (!modern) {
11811198
dom_reconcile_ns(nodep->doc, newchild);
@@ -1265,8 +1282,7 @@ static void dom_node_append_child_legacy(zval *return_value, dom_object *intern,
12651282
}
12661283

12671284
if (child->doc == NULL && nodep->doc != NULL) {
1268-
childobj->document = intern->document;
1269-
php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
1285+
dom_set_document_pointers(child, nodep->doc, intern->document);
12701286
}
12711287

12721288
if (child->parent != NULL){
@@ -1275,9 +1291,6 @@ static void dom_node_append_child_legacy(zval *return_value, dom_object *intern,
12751291

12761292
if (child->type == XML_TEXT_NODE && nodep->last != NULL && nodep->last->type == XML_TEXT_NODE) {
12771293
child->parent = nodep;
1278-
if (child->doc == NULL) {
1279-
xmlSetTreeDoc(child, nodep->doc);
1280-
}
12811294
new_child = child;
12821295
if (nodep->children == NULL) {
12831296
nodep->children = child;

‎ext/dom/tests/gh16150.phpt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-16150 (Use after free in php_dom.c)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
function test($fname) {
9+
$e1 = new DOMElement("E1");
10+
$e2 = new DOMElement("E2");
11+
$e3 = new DOMElement("E3");
12+
$doc = new DOMDocument(); // Must be placed here so it is destroyed first
13+
$doc->{$fname}($e3);
14+
$e2->append($e1);
15+
$e3->{$fname}($e2);
16+
echo $doc->saveXML();
17+
}
18+
19+
test('appendChild');
20+
test('insertBefore');
21+
22+
?>
23+
--EXPECT--
24+
<?xml version="1.0"?>
25+
<E3><E2><E1/></E2></E3>
26+
<?xml version="1.0"?>
27+
<E3><E2><E1/></E2></E3>

‎ext/dom/tests/gh16152.phpt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-16152 (Memory leak in DOMProcessingInstruction/DOMDocument)
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
8+
function test($fname) {
9+
$doc = new DOMDocument();
10+
$instr = new DOMProcessingInstruction("tr", "r");
11+
$frag = new DOMDocumentFragment();
12+
$frag2 = new DOMDocumentFragment();
13+
$frag2->append($instr);
14+
$frag->append($frag2);
15+
$doc->{$fname}($frag);
16+
echo $doc->saveXML();
17+
}
18+
19+
test('insertBefore');
20+
test('appendChild');
21+
22+
?>
23+
--EXPECT--
24+
<?xml version="1.0"?>
25+
<?tr r?>
26+
<?xml version="1.0"?>
27+
<?tr r?>

0 commit comments

Comments
 (0)