Skip to content

Fix bugs GH-16150 and GH-16152: intern document mismanagement #16178

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix bugs GH-16150 and GH-16152: intern document mismanagement #16178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 62 additions & 30 deletions ext/dom/node.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,11 +820,62 @@ int dom_node_text_content_write(dom_object *obj, zval *newval)

/* }}} */

static xmlNodePtr _php_dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlNodePtr nextsib, xmlNodePtr fragment, dom_object *intern, dom_object *childobj) /* {{{ */
/* Returns true if the node was changed, false otherwise. */
static bool dom_set_document_ref_obj_single(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
{
xmlNodePtr newchild, node;
dom_object *childobj = php_dom_object_get_data(node);
if (childobj && !childobj->document) {
childobj->document = document;
document->refcount++;
return true;
}
return false;
}

/* TODO: on 8.4 replace the loop with the tree walk helper function. */
static void dom_set_document_pointers(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document)
{
/* Applies the document to the entire subtree. */
xmlSetTreeDoc(node, doc);

if (!dom_set_document_ref_obj_single(node, doc, document)) {
return;
}

xmlNodePtr base = node;
node = node->children;
while (node != NULL) {
ZEND_ASSERT(node != base);

if (!dom_set_document_ref_obj_single(node, doc, document)) {
break;
}

if (node->type == XML_ELEMENT_NODE) {
if (node->children) {
node = node->children;
continue;
}
}

if (node->next) {
node = node->next;
} else {
/* Go upwards, until we find a parent node with a next sibling, or until we hit the base. */
do {
node = node->parent;
if (node == base) {
return;
}
} while (node->next == NULL);
node = node->next;
}
}
}

newchild = fragment->children;
static xmlNodePtr _php_dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib, xmlNodePtr nextsib, xmlNodePtr fragment, dom_object *intern, dom_object *childobj) /* {{{ */
{
xmlNodePtr newchild = fragment->children;

if (newchild) {
if (prevsib == NULL) {
Expand All @@ -840,17 +891,10 @@ static xmlNodePtr _php_dom_insert_fragment(xmlNodePtr nodep, xmlNodePtr prevsib,
nextsib->prev = fragment->last;
}

node = newchild;
/* Assign parent node pointer */
xmlNodePtr node = newchild;
while (node != NULL) {
node->parent = nodep;
if (node->doc != nodep->doc) {
xmlSetTreeDoc(node, nodep->doc);
if (node->_private != NULL) {
childobj = node->_private;
childobj->document = intern->document;
php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
}
}
if (node == fragment->last) {
break;
}
Expand Down Expand Up @@ -930,8 +974,7 @@ PHP_METHOD(DOMNode, insertBefore)
}

if (child->doc == NULL && parentp->doc != NULL) {
childobj->document = intern->document;
php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
dom_set_document_pointers(child, parentp->doc, intern->document);
}

php_libxml_invalidate_node_list_cache(intern->document);
Expand All @@ -949,9 +992,6 @@ PHP_METHOD(DOMNode, insertBefore)

if (child->type == XML_TEXT_NODE && (refp->type == XML_TEXT_NODE ||
(refp->prev != NULL && refp->prev->type == XML_TEXT_NODE))) {
if (child->doc == NULL) {
xmlSetTreeDoc(child, parentp->doc);
}
new_child = child;
new_child->parent = refp->parent;
new_child->next = refp;
Expand Down Expand Up @@ -1003,9 +1043,6 @@ PHP_METHOD(DOMNode, insertBefore)
}
if (child->type == XML_TEXT_NODE && parentp->last != NULL && parentp->last->type == XML_TEXT_NODE) {
child->parent = parentp;
if (child->doc == NULL) {
xmlSetTreeDoc(child, parentp->doc);
}
new_child = child;
if (parentp->children == NULL) {
parentp->children = child;
Expand Down Expand Up @@ -1111,6 +1148,10 @@ PHP_METHOD(DOMNode, replaceChild)
RETURN_FALSE;
}

if (newchild->doc == NULL && nodep->doc != NULL) {
dom_set_document_pointers(newchild, nodep->doc, intern->document);
}

if (newchild->type == XML_DOCUMENT_FRAG_NODE) {
xmlNodePtr prevsib, nextsib;
prevsib = oldchild->prev;
Expand All @@ -1127,11 +1168,6 @@ PHP_METHOD(DOMNode, replaceChild)
xmlDtdPtr intSubset = xmlGetIntSubset(nodep->doc);
replacedoctype = (intSubset == (xmlDtd *) oldchild);

if (newchild->doc == NULL && nodep->doc != NULL) {
xmlSetTreeDoc(newchild, nodep->doc);
newchildobj->document = intern->document;
php_libxml_increment_doc_ref((php_libxml_node_object *)newchildobj, NULL);
}
xmlReplaceNode(oldchild, newchild);
dom_reconcile_ns(nodep->doc, newchild);

Expand Down Expand Up @@ -1216,8 +1252,7 @@ PHP_METHOD(DOMNode, appendChild)
}

if (child->doc == NULL && nodep->doc != NULL) {
childobj->document = intern->document;
php_libxml_increment_doc_ref((php_libxml_node_object *)childobj, NULL);
dom_set_document_pointers(child, nodep->doc, intern->document);
}

if (child->parent != NULL){
Expand All @@ -1226,9 +1261,6 @@ PHP_METHOD(DOMNode, appendChild)

if (child->type == XML_TEXT_NODE && nodep->last != NULL && nodep->last->type == XML_TEXT_NODE) {
child->parent = nodep;
if (child->doc == NULL) {
xmlSetTreeDoc(child, nodep->doc);
}
new_child = child;
if (nodep->children == NULL) {
nodep->children = child;
Expand Down
27 changes: 27 additions & 0 deletions ext/dom/tests/gh16150.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
GH-16150 (Use after free in php_dom.c)
--EXTENSIONS--
dom
--FILE--
<?php

function test($fname) {
$e1 = new DOMElement("E1");
$e2 = new DOMElement("E2");
$e3 = new DOMElement("E3");
$doc = new DOMDocument(); // Must be placed here so it is destroyed first
$doc->{$fname}($e3);
$e2->append($e1);
$e3->{$fname}($e2);
echo $doc->saveXML();
}

test('appendChild');
test('insertBefore');

?>
--EXPECT--
<?xml version="1.0"?>
<E3><E2><E1/></E2></E3>
<?xml version="1.0"?>
<E3><E2><E1/></E2></E3>
27 changes: 27 additions & 0 deletions ext/dom/tests/gh16152.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
GH-16152 (Memory leak in DOMProcessingInstruction/DOMDocument)
--EXTENSIONS--
dom
--FILE--
<?php

function test($fname) {
$doc = new DOMDocument();
$instr = new DOMProcessingInstruction("tr", "r");
$frag = new DOMDocumentFragment();
$frag2 = new DOMDocumentFragment();
$frag2->append($instr);
$frag->append($frag2);
$doc->{$fname}($frag);
echo $doc->saveXML();
}

test('insertBefore');
test('appendChild');

?>
--EXPECT--
<?xml version="1.0"?>
<?tr r?>
<?xml version="1.0"?>
<?tr r?>
Loading