Skip to content

Fix GH-11288 and GH-11289 and GH-11290 and GH-9142: DOMExceptions and segfaults with replaceWith #11299

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 3 commits into from
Closed

Fix GH-11288 and GH-11289 and GH-11290 and GH-9142: DOMExceptions and segfaults with replaceWith #11299

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
382101e
Fix GH-11288 and GH-11289 and GH-11290 and GH-9142: DOMException with…
nielsdos May 22, 2023
174a0f3
Fix Travis failure due to different formatting (?)
nielsdos May 24, 2023
53e3feb
Fixup spec step 2
nielsdos May 25, 2023
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
188 changes: 103 additions & 85 deletions ext/dom/parentnode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,23 @@ int dom_parent_node_child_element_count(dom_object *obj, zval *retval)
}
/* }}} */

static bool dom_is_node_in_list(const zval *nodes, int nodesc, const xmlNodePtr node_to_find)
{
for (int i = 0; i < nodesc; i++) {
if (Z_TYPE(nodes[i]) == IS_OBJECT) {
const zend_class_entry *ce = Z_OBJCE(nodes[i]);

if (instanceof_function(ce, dom_node_class_entry)) {
if (dom_object_get_node(Z_DOMOBJ_P(nodes + i)) == node_to_find) {
return true;
}
}
}
}

return false;
}

xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNode, zval *nodes, int nodesc)
{
int i;
Expand Down Expand Up @@ -177,17 +194,16 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
goto hierarchy_request_err;
}

/*
* xmlNewDocText function will always returns same address to the second parameter if the parameters are greater than or equal to three.
* If it's text, that's fine, but if it's an object, it can cause invalid pointer because many new nodes point to the same memory address.
* So we must copy the new node to avoid this situation.
*/
if (nodesc > 1) {
/* Citing from the docs (https://gnome.pages.gitlab.gnome.org/libxml2/devhelp/libxml2-tree.html#xmlAddChild):
* "Add a new node to @parent, at the end of the child (or property) list merging adjacent TEXT nodes (in which case @cur is freed)".
* So we must take a copy if this situation arises to prevent a use-after-free. */
bool will_free = newNode->type == XML_TEXT_NODE && fragment->last && fragment->last->type == XML_TEXT_NODE;
if (will_free) {
newNode = xmlCopyNode(newNode, 1);
}

if (!xmlAddChild(fragment, newNode)) {
if (nodesc > 1) {
if (will_free) {
xmlFreeNode(newNode);
}
goto hierarchy_request_err;
Expand Down Expand Up @@ -303,25 +319,64 @@ void dom_parent_node_prepend(dom_object *context, zval *nodes, int nodesc)
xmlFree(fragment);
}

static void dom_pre_insert(xmlNodePtr insertion_point, xmlNodePtr parentNode, xmlNodePtr newchild, xmlNodePtr fragment)
{
if (!insertion_point) {
/* Place it as last node */
if (parentNode->children) {
/* There are children */
fragment->last->prev = parentNode->last;
newchild->prev = parentNode->last->prev;
parentNode->last->next = newchild;
} else {
/* No children, because they moved out when they became a fragment */
parentNode->children = newchild;
parentNode->last = newchild;
}
} else {
/* Insert fragment before insertion_point */
fragment->last->next = insertion_point;
if (insertion_point->prev) {
insertion_point->prev->next = newchild;
newchild->prev = insertion_point->prev;
}
insertion_point->prev = newchild;
if (parentNode->children == insertion_point) {
parentNode->children = newchild;
}
}
}

void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
{
/* Spec link: https://dom.spec.whatwg.org/#dom-childnode-after */

xmlNode *prevsib = dom_object_get_node(context);
xmlNodePtr newchild, parentNode;
xmlNode *fragment, *nextsib;
xmlNode *fragment;
xmlDoc *doc;
bool afterlastchild;

int stricterror = dom_get_strict_error(context->document);

if (!prevsib->parent) {
php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, stricterror);
/* Spec step 1 */
parentNode = prevsib->parent;
/* Spec step 2 */
if (!parentNode) {
int stricterror = dom_get_strict_error(context->document);
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
return;
}

/* Spec step 3: find first following child not in nodes; otherwise null */
xmlNodePtr viable_next_sibling = prevsib->next;
while (viable_next_sibling) {
if (!dom_is_node_in_list(nodes, nodesc, viable_next_sibling)) {
break;
}
viable_next_sibling = viable_next_sibling->next;
}

doc = prevsib->doc;
parentNode = prevsib->parent;
nextsib = prevsib->next;
afterlastchild = (nextsib == NULL);

/* Spec step 4: convert nodes into fragment */
fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);

if (fragment == NULL) {
Expand All @@ -331,40 +386,9 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
newchild = fragment->children;

if (newchild) {
/* first node and last node are both both parameters to DOMElement::after() method so nextsib and prevsib are null. */
if (!parentNode->children) {
prevsib = nextsib = NULL;
} else if (afterlastchild) {
/*
* The new node will be inserted after last node, prevsib is last node.
* The first node is the parameter to DOMElement::after() if parentNode->children == prevsib is true
* and prevsib does not change, otherwise prevsib is parentNode->last (first node).
*/
prevsib = parentNode->children == prevsib ? prevsib : parentNode->last;
} else {
/*
* The new node will be inserted after first node, prevsib is first node.
* The first node is not the parameter to DOMElement::after() if parentNode->children == prevsib is true
* and prevsib does not change otherwise prevsib is null to mean that parentNode->children is the new node.
*/
prevsib = parentNode->children == prevsib ? prevsib : NULL;
}

if (prevsib) {
fragment->last->next = prevsib->next;
if (prevsib->next) {
prevsib->next->prev = fragment->last;
}
prevsib->next = newchild;
} else {
parentNode->children = newchild;
if (nextsib) {
fragment->last->next = nextsib;
nextsib->prev = fragment->last;
}
}
/* Step 5: place fragment into the parent before viable_next_sibling */
dom_pre_insert(viable_next_sibling, parentNode, newchild, fragment);

newchild->prev = prevsib;
dom_fragment_assign_parent_node(parentNode, fragment);
dom_reconcile_ns(doc, newchild);
}
Expand All @@ -374,17 +398,34 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)

void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
{
/* Spec link: https://dom.spec.whatwg.org/#dom-childnode-before */

xmlNode *nextsib = dom_object_get_node(context);
xmlNodePtr newchild, prevsib, parentNode;
xmlNode *fragment, *afternextsib;
xmlNodePtr newchild, parentNode;
xmlNode *fragment;
xmlDoc *doc;
bool beforefirstchild;

doc = nextsib->doc;
prevsib = nextsib->prev;
afternextsib = nextsib->next;
/* Spec step 1 */
parentNode = nextsib->parent;
beforefirstchild = !prevsib;
/* Spec step 2 */
if (!parentNode) {
int stricterror = dom_get_strict_error(context->document);
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
return;
}

/* Spec step 3: find first following child not in nodes; otherwise null */
xmlNodePtr viable_previous_sibling = nextsib->prev;
while (viable_previous_sibling) {
if (!dom_is_node_in_list(nodes, nodesc, viable_previous_sibling)) {
break;
}
viable_previous_sibling = viable_previous_sibling->prev;
}

doc = nextsib->doc;

/* Spec step 4: convert nodes into fragment */
fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);

if (fragment == NULL) {
Expand All @@ -394,37 +435,14 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
newchild = fragment->children;

if (newchild) {
/* first node and last node are both both parameters to DOMElement::before() method so nextsib is null. */
if (!parentNode->children) {
nextsib = NULL;
} else if (beforefirstchild) {
/*
* The new node will be inserted before first node, nextsib is first node and afternextsib is last node.
* The first node is not the parameter to DOMElement::before() if parentNode->children == nextsib is true
* and nextsib does not change, otherwise nextsib is the last node.
*/
nextsib = parentNode->children == nextsib ? nextsib : afternextsib;
} else {
/*
* The new node will be inserted before last node, prevsib is first node and nestsib is last node.
* The first node is not the parameter to DOMElement::before() if parentNode->children == prevsib is true
* but last node may be, so use prevsib->next to determine the value of nextsib, otherwise nextsib does not change.
*/
nextsib = parentNode->children == prevsib ? prevsib->next : nextsib;
}

if (parentNode->children == nextsib) {
parentNode->children = newchild;
/* Step 5: if viable_previous_sibling is null, set it to the parent's first child, otherwise viable_previous_sibling's next sibling */
if (!viable_previous_sibling) {
viable_previous_sibling = parentNode->children;
} else {
prevsib->next = newchild;
}

fragment->last->next = nextsib;
if (nextsib) {
nextsib->prev = fragment->last;
viable_previous_sibling = viable_previous_sibling->next;
}

newchild->prev = prevsib;
/* Step 6: place fragment into the parent after viable_previous_sibling */
dom_pre_insert(viable_previous_sibling, parentNode, newchild, fragment);

dom_fragment_assign_parent_node(parentNode, fragment);
dom_reconcile_ns(doc, newchild);
Expand Down
Loading