diff --git a/NEWS b/NEWS index 1a65395be50..33697acf622 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,8 @@ PHP NEWS . Add missing hierarchy checks to replaceChild. (nielsdos) . Fixed bug GH-16465 (Heap buffer overflow in DOMNode->getElementByTagName). (nielsdos) + . Fixed bug GH-16336 (Attribute intern document mismanagement). (nielsdos) + . Fixed bug GH-16338 (Null-dereference in ext/dom/node.c). (nielsdos) - EXIF: . Fixed bug GH-16409 (Segfault in exif_thumbnail when not dealing with a diff --git a/ext/dom/element.c b/ext/dom/element.c index 6fcaee5888e..64c53e4a2d8 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -734,9 +734,8 @@ static void dom_element_set_attribute_node_common(INTERNAL_FUNCTION_PARAMETERS, xmlUnlinkNode((xmlNodePtr) attrp); } - if (attrp->doc == NULL && nodep->doc != NULL) { - attrobj->document = intern->document; - php_libxml_increment_doc_ref((php_libxml_node_object *)attrobj, NULL); + if (attrp->doc == NULL && nodep->doc != NULL && intern->document != NULL) { + dom_set_document_ref_pointers_attr(attrp, intern->document); } xmlAddChild(nodep, (xmlNodePtr) attrp); diff --git a/ext/dom/node.c b/ext/dom/node.c index bb35671985d..1a433468ede 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -740,11 +740,14 @@ zend_result dom_node_text_content_write(dom_object *obj, zval *newval) /* }}} */ -/* 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) +/* Returns true if the node had the same document reference, false otherwise. */ +static bool dom_set_document_ref_obj_single(xmlNodePtr node, php_libxml_ref_obj *document) { dom_object *childobj = php_dom_object_get_data(node); - if (childobj && !childobj->document) { + if (!childobj) { + return true; + } + if (!childobj->document) { childobj->document = document; document->refcount++; return true; @@ -752,24 +755,46 @@ static bool dom_set_document_ref_obj_single(xmlNodePtr node, xmlDocPtr doc, php_ return false; } -static void dom_set_document_pointers(xmlNodePtr node, xmlDocPtr doc, php_libxml_ref_obj *document) +void dom_set_document_ref_pointers_attr(xmlAttrPtr attr, php_libxml_ref_obj *document) { - /* Applies the document to the entire subtree. */ - xmlSetTreeDoc(node, doc); + ZEND_ASSERT(document != NULL); - if (!dom_set_document_ref_obj_single(node, doc, document)) { + dom_set_document_ref_obj_single((xmlNodePtr) attr, document); + for (xmlNodePtr attr_child = attr->children; attr_child; attr_child = attr_child->next) { + dom_set_document_ref_obj_single(attr_child, document); + } +} + +static bool dom_set_document_ref_pointers_node(xmlNodePtr node, php_libxml_ref_obj *document) +{ + ZEND_ASSERT(document != NULL); + + if (!dom_set_document_ref_obj_single(node, document)) { + return false; + } + + if (node->type == XML_ELEMENT_NODE) { + for (xmlAttrPtr attr = node->properties; attr; attr = attr->next) { + dom_set_document_ref_pointers_attr(attr, document); + } + } + + return true; +} + +void dom_set_document_ref_pointers(xmlNodePtr node, php_libxml_ref_obj *document) +{ + if (!document) { + return; + } + + if (!dom_set_document_ref_pointers_node(node, 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; - } - + while (node != NULL && dom_set_document_ref_pointers_node(node, document)) { node = php_dom_next_in_tree_order(node, base); } } @@ -860,7 +885,7 @@ static void dom_node_insert_before_legacy(zval *return_value, zval *ref, dom_obj } if (child->doc == NULL && parentp->doc != NULL) { - dom_set_document_pointers(child, parentp->doc, intern->document); + dom_set_document_ref_pointers(child, intern->document); } php_libxml_invalidate_node_list_cache(intern->document); @@ -1167,7 +1192,7 @@ static void dom_node_replace_child(INTERNAL_FUNCTION_PARAMETERS, bool modern) } if (newchild->doc == NULL && nodep->doc != NULL) { - dom_set_document_pointers(newchild, nodep->doc, intern->document); + dom_set_document_ref_pointers(newchild, intern->document); } if (newchild->type == XML_DOCUMENT_FRAG_NODE) { @@ -1275,7 +1300,7 @@ static void dom_node_append_child_legacy(zval *return_value, dom_object *intern, } if (child->doc == NULL && nodep->doc != NULL) { - dom_set_document_pointers(child, nodep->doc, intern->document); + dom_set_document_ref_pointers(child, intern->document); } if (child->parent != NULL){ diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index bf671577980..851bc14d125 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -181,6 +181,8 @@ bool dom_compare_value(const xmlAttr *attr, const xmlChar *value); void dom_attr_value_will_change(dom_object *obj, xmlAttrPtr attrp); bool php_dom_create_nullable_object(xmlNodePtr obj, zval *return_value, dom_object *domobj); xmlNodePtr dom_clone_node(php_dom_libxml_ns_mapper *ns_mapper, xmlNodePtr node, xmlDocPtr doc, bool recursive); +void dom_set_document_ref_pointers(xmlNodePtr node, php_libxml_ref_obj *document); +void dom_set_document_ref_pointers_attr(xmlAttrPtr attr, php_libxml_ref_obj *document); typedef enum { DOM_LOAD_STRING = 0, diff --git a/ext/dom/tests/gh16336_1.phpt b/ext/dom/tests/gh16336_1.phpt new file mode 100644 index 00000000000..3488a7f1ee9 --- /dev/null +++ b/ext/dom/tests/gh16336_1.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16336 (Attribute intern document mismanagement) +--EXTENSIONS-- +dom +--FILE-- +appendChild($elem); +$elem->setAttributeNode($attr); +echo $attr->firstChild->textContent; + +?> +--EXPECT-- +j diff --git a/ext/dom/tests/gh16336_2.phpt b/ext/dom/tests/gh16336_2.phpt new file mode 100644 index 00000000000..0a4801f6087 --- /dev/null +++ b/ext/dom/tests/gh16336_2.phpt @@ -0,0 +1,19 @@ +--TEST-- +GH-16336 (Attribute intern document mismanagement) +--EXTENSIONS-- +dom +--FILE-- +setAttributeNode($attr); +$doc->appendChild($elem); +echo $attr->firstChild->textContent; + +?> +--EXPECT-- +j diff --git a/ext/dom/tests/gh16338.phpt b/ext/dom/tests/gh16338.phpt new file mode 100644 index 00000000000..7346f62107e --- /dev/null +++ b/ext/dom/tests/gh16338.phpt @@ -0,0 +1,27 @@ +--TEST-- +GH-16338 (Null-dereference in ext/dom/node.c) +--EXTENSIONS-- +dom +--CREDITS-- +chibinz +--FILE-- +prepend($com); +$com->before("Z"); +$com->before($com2); +$com2->after($elem); +$doc->insertBefore($elem2); +$elem->insertBefore($ref); + +echo $doc->saveXML(); +?> +--EXPECT-- + +Zo&G;