diff --git a/ext/dom/node.c b/ext/dom/node.c index e061b13e0137b..48e3c5cc2a747 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -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) { @@ -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; } @@ -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); @@ -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; @@ -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; @@ -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; @@ -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); @@ -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){ @@ -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; diff --git a/ext/dom/tests/gh16150.phpt b/ext/dom/tests/gh16150.phpt new file mode 100644 index 0000000000000..dd1096d6812cc --- /dev/null +++ b/ext/dom/tests/gh16150.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-16150 (Use after free in php_dom.c) +--EXTENSIONS-- +dom +--FILE-- +{$fname}($e3); + $e2->append($e1); + $e3->{$fname}($e2); + echo $doc->saveXML(); +} + +test('appendChild'); +test('insertBefore'); + +?> +--EXPECT-- + + + + diff --git a/ext/dom/tests/gh16152.phpt b/ext/dom/tests/gh16152.phpt new file mode 100644 index 0000000000000..604980b855e2f --- /dev/null +++ b/ext/dom/tests/gh16152.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-16152 (Memory leak in DOMProcessingInstruction/DOMDocument) +--EXTENSIONS-- +dom +--FILE-- +append($instr); + $frag->append($frag2); + $doc->{$fname}($frag); + echo $doc->saveXML(); +} + +test('insertBefore'); +test('appendChild'); + +?> +--EXPECT-- + + + +