diff --git a/ext/dom/node.c b/ext/dom/node.c index a42dfedc32a5..686c16f34b2f 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -66,7 +66,7 @@ bool php_dom_is_node_connected(const xmlNode *node) if (node->type == XML_DOCUMENT_NODE || node->type == XML_HTML_DOCUMENT_NODE) { return true; } - node = node->parent; + node = php_dom_parent_node(node); } while (node != NULL); return false; } @@ -244,7 +244,7 @@ static zend_result dom_node_parent_get(dom_object *obj, zval *retval, bool only_ { DOM_PROP_NODE(xmlNodePtr, nodep, obj); - xmlNodePtr nodeparent = nodep->parent; + xmlNodePtr nodeparent = php_dom_parent_node(nodep); if (!nodeparent || (only_element && nodeparent->type != XML_ELEMENT_NODE)) { ZVAL_NULL(retval); return SUCCESS; @@ -2412,7 +2412,7 @@ static bool dom_node_contains(xmlNodePtr thisp, xmlNodePtr otherp) if (otherp == thisp) { return true; } - otherp = otherp->parent; + otherp = php_dom_parent_node(otherp); } while (otherp); return false; @@ -2470,7 +2470,7 @@ PHP_METHOD(Dom_Node, contains) PHP_METHOD(DOMNode, getRootNode) { zval *id; - xmlNodePtr thisp; + xmlNodePtr thisp, tmp; dom_object *intern; /* Unused now because we don't support the shadow DOM nodes. Options only influence shadow DOM nodes. */ zval *options; @@ -2482,8 +2482,8 @@ PHP_METHOD(DOMNode, getRootNode) DOM_GET_THIS_OBJ(thisp, id, xmlNodePtr, intern); - while (thisp->parent) { - thisp = thisp->parent; + while ((tmp = php_dom_parent_node(thisp))) { + thisp = tmp; } DOM_RET_OBJ(thisp, intern); @@ -2559,9 +2559,9 @@ static void dom_node_compare_document_position(INTERNAL_FUNCTION_PARAMETERS, zen } bool node2_is_ancestor_of_node1 = false; size_t node1_depth = 0; - xmlNodePtr node1_root = node1; - while (node1_root->parent) { - node1_root = node1_root->parent; + xmlNodePtr node1_root = node1, tmp; + while ((tmp = php_dom_parent_node(node1_root))) { + node1_root = tmp; if (node1_root == node2) { node2_is_ancestor_of_node1 = true; } @@ -2570,8 +2570,8 @@ static void dom_node_compare_document_position(INTERNAL_FUNCTION_PARAMETERS, zen bool node1_is_ancestor_of_node2 = false; size_t node2_depth = 0; xmlNodePtr node2_root = node2; - while (node2_root->parent) { - node2_root = node2_root->parent; + while ((tmp = php_dom_parent_node(node2_root))) { + node2_root = tmp; if (node2_root == node1) { node1_is_ancestor_of_node2 = true; } diff --git a/ext/dom/parentnode/css_selectors.c b/ext/dom/parentnode/css_selectors.c index 37f9d698e856..358a425db729 100644 --- a/ext/dom/parentnode/css_selectors.c +++ b/ext/dom/parentnode/css_selectors.c @@ -200,7 +200,7 @@ static const xmlNode *dom_query_closest( ret = current; break; } - current = current->parent; + current = php_dom_parent_node(current); } } diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index 13f49879bb38..8fe5d57e7f49 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -247,6 +247,11 @@ xmlNodePtr dom_nodelist_iter_start_first_child(xmlNodePtr nodep); __ptr = (__prtype)((php_libxml_node_ptr *)__intern->ptr)->node; \ } +static zend_always_inline xmlNodePtr php_dom_parent_node(const xmlNode *nodep) +{ + return nodep->type == XML_DOCUMENT_FRAG_NODE ? NULL : nodep->parent; +} + static zend_always_inline bool php_dom_is_cache_tag_stale_from_doc_ptr(const php_libxml_cache_tag *cache_tag, const php_libxml_ref_obj *doc_ptr) { ZEND_ASSERT(doc_ptr != NULL); diff --git a/ext/dom/private_data.c b/ext/dom/private_data.c index 66b3f4f87f29..d908ad329251 100644 --- a/ext/dom/private_data.c +++ b/ext/dom/private_data.c @@ -72,9 +72,7 @@ void php_dom_private_data_destroy(php_dom_private_data *data) static void php_dom_free_templated_content(php_dom_private_data *private_data, xmlNodePtr base) { - /* Note: it's not possible to obtain a userland reference to these yet, so we can just free them without worrying - * about their proxies. - * Note 2: it's possible to have nested template content. */ + /* Note: it's possible to have nested template content. */ if (zend_hash_num_elements(private_data->template_fragments) > 0) { /* There's more templated content, try to free it. */ @@ -88,7 +86,13 @@ static void php_dom_free_templated_content(php_dom_private_data *private_data, x } } - xmlFreeNode(base); + php_libxml_node_free_list(base->children); + + if (!base->_private) { + xmlFreeNode(base); + } else { + base->parent = NULL; + } } void php_dom_add_templated_content(php_dom_private_data *private_data, const xmlNode *template_node, xmlNodePtr fragment) diff --git a/ext/dom/tests/modern/html/interactions/gh23334.phpt b/ext/dom/tests/modern/html/interactions/gh23334.phpt new file mode 100644 index 000000000000..82a86b6c1bf8 --- /dev/null +++ b/ext/dom/tests/modern/html/interactions/gh23334.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-23334 (UAF when template contents are freed while userland references remain) +--EXTENSIONS-- +dom +--FILE-- +