Skip to content

Commit 2d6decc

Browse files
NathanFreemannielsdos
NathanFreeman
authored and
nielsdos
committed
Fix bug #80602: Segfault when using DOMChildNode::before()
This furthermore fixes the logic error explained in #8729 (comment) Closes GH-10682.
1 parent d9df750 commit 2d6decc

File tree

4 files changed

+441
-9
lines changed

4 files changed

+441
-9
lines changed

‎NEWS

+4
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ PHP NEWS
2424
. Fixed bug GH-10583 (DateTime modify with tz pattern should not update
2525
linked timezone). (Derick)
2626

27+
- DOM:
28+
. Fixed bug #80602 (Segfault when using DOMChildNode::before()).
29+
(Nathan Freeman)
30+
2731
- FPM:
2832
. Fixed bug GH-10611 (fpm_env_init_main leaks environ). (nielsdos)
2933
. Destroy file_handle in fpm_main. (Jakub Zelenka, nielsdos)

‎ext/dom/parentnode.c

+81-9
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,20 @@ xmlNode* dom_zvals_to_fragment(php_libxml_ref_obj *document, xmlNode *contextNod
181181
return NULL;
182182
}
183183

184+
/*
185+
* xmlNewDocText function will always returns same address to the second parameter if the parameters are greater than or equal to three.
186+
* 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.
187+
* So we must copy the new node to avoid this situation.
188+
*/
189+
if (nodesc > 1) {
190+
newNode = xmlCopyNode(newNode, 1);
191+
}
192+
184193
if (!xmlAddChild(fragment, newNode)) {
185194
xmlFree(fragment);
195+
if (nodesc > 1) {
196+
xmlFreeNode(newNode);
197+
}
186198

187199
php_dom_throw_error(HIERARCHY_REQUEST_ERR, stricterror);
188200
return NULL;
@@ -302,7 +314,9 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
302314
{
303315
xmlNode *prevsib = dom_object_get_node(context);
304316
xmlNodePtr newchild, parentNode;
305-
xmlNode *fragment;
317+
xmlNode *fragment, *nextsib;
318+
xmlDoc *doc;
319+
bool afterlastchild;
306320

307321
int stricterror = dom_get_strict_error(context->document);
308322

@@ -311,7 +325,10 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
311325
return;
312326
}
313327

328+
doc = prevsib->doc;
314329
parentNode = prevsib->parent;
330+
nextsib = prevsib->next;
331+
afterlastchild = (nextsib == NULL);
315332
fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
316333

317334
if (fragment == NULL) {
@@ -321,13 +338,42 @@ void dom_parent_node_after(dom_object *context, zval *nodes, int nodesc)
321338
newchild = fragment->children;
322339

323340
if (newchild) {
324-
fragment->last->next = prevsib->next;
325-
prevsib->next = newchild;
341+
/* first node and last node are both both parameters to DOMElement::after() method so nextsib and prevsib are null. */
342+
if (!parentNode->children) {
343+
prevsib = nextsib = NULL;
344+
} else if (afterlastchild) {
345+
/*
346+
* The new node will be inserted after last node, prevsib is last node.
347+
* The first node is the parameter to DOMElement::after() if parentNode->children == prevsib is true
348+
* and prevsib does not change, otherwise prevsib is parentNode->last (first node).
349+
*/
350+
prevsib = parentNode->children == prevsib ? prevsib : parentNode->last;
351+
} else {
352+
/*
353+
* The new node will be inserted after first node, prevsib is first node.
354+
* The first node is not the parameter to DOMElement::after() if parentNode->children == prevsib is true
355+
* and prevsib does not change otherwise prevsib is null to mean that parentNode->children is the new node.
356+
*/
357+
prevsib = parentNode->children == prevsib ? prevsib : NULL;
358+
}
326359

327-
newchild->prev = prevsib;
360+
if (prevsib) {
361+
fragment->last->next = prevsib->next;
362+
if (prevsib->next) {
363+
prevsib->next->prev = fragment->last;
364+
}
365+
prevsib->next = newchild;
366+
} else {
367+
parentNode->children = newchild;
368+
if (nextsib) {
369+
fragment->last->next = nextsib;
370+
nextsib->prev = fragment->last;
371+
}
372+
}
328373

374+
newchild->prev = prevsib;
329375
dom_fragment_assign_parent_node(parentNode, fragment);
330-
dom_reconcile_ns(prevsib->doc, newchild);
376+
dom_reconcile_ns(doc, newchild);
331377
}
332378

333379
xmlFree(fragment);
@@ -337,10 +383,15 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
337383
{
338384
xmlNode *nextsib = dom_object_get_node(context);
339385
xmlNodePtr newchild, prevsib, parentNode;
340-
xmlNode *fragment;
386+
xmlNode *fragment, *afternextsib;
387+
xmlDoc *doc;
388+
bool beforefirstchild;
341389

390+
doc = nextsib->doc;
342391
prevsib = nextsib->prev;
392+
afternextsib = nextsib->next;
343393
parentNode = nextsib->parent;
394+
beforefirstchild = !prevsib;
344395
fragment = dom_zvals_to_fragment(context->document, parentNode, nodes, nodesc);
345396

346397
if (fragment == NULL) {
@@ -350,19 +401,40 @@ void dom_parent_node_before(dom_object *context, zval *nodes, int nodesc)
350401
newchild = fragment->children;
351402

352403
if (newchild) {
404+
/* first node and last node are both both parameters to DOMElement::before() method so nextsib is null. */
405+
if (!parentNode->children) {
406+
nextsib = NULL;
407+
} else if (beforefirstchild) {
408+
/*
409+
* The new node will be inserted before first node, nextsib is first node and afternextsib is last node.
410+
* The first node is not the parameter to DOMElement::before() if parentNode->children == nextsib is true
411+
* and nextsib does not change, otherwise nextsib is the last node.
412+
*/
413+
nextsib = parentNode->children == nextsib ? nextsib : afternextsib;
414+
} else {
415+
/*
416+
* The new node will be inserted before last node, prevsib is first node and nestsib is last node.
417+
* The first node is not the parameter to DOMElement::before() if parentNode->children == prevsib is true
418+
* but last node may be, so use prevsib->next to determine the value of nextsib, otherwise nextsib does not change.
419+
*/
420+
nextsib = parentNode->children == prevsib ? prevsib->next : nextsib;
421+
}
422+
353423
if (parentNode->children == nextsib) {
354424
parentNode->children = newchild;
355425
} else {
356426
prevsib->next = newchild;
357427
}
428+
358429
fragment->last->next = nextsib;
359-
nextsib->prev = fragment->last;
430+
if (nextsib) {
431+
nextsib->prev = fragment->last;
432+
}
360433

361434
newchild->prev = prevsib;
362435

363436
dom_fragment_assign_parent_node(parentNode, fragment);
364-
365-
dom_reconcile_ns(nextsib->doc, newchild);
437+
dom_reconcile_ns(doc, newchild);
366438
}
367439

368440
xmlFree(fragment);

‎ext/dom/tests/bug80602.phpt

+178
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
--TEST--
2+
Bug #80602 (Segfault when using DOMChildNode::before())
3+
--FILE--
4+
<?php
5+
declare(strict_types=1);
6+
7+
$doc = new \DOMDocument();
8+
$doc->loadXML('<a>foo<last/></a>');
9+
$target = $doc->documentElement->firstChild;
10+
$target->before($target);
11+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
12+
13+
$doc = new \DOMDocument();
14+
$doc->loadXML('<a>foo<last/></a>');
15+
$target = $doc->documentElement->lastChild;
16+
$target->before($target);
17+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
18+
19+
20+
$doc = new \DOMDocument();
21+
$doc->loadXML('<a>foo<last/></a>');
22+
$target = $doc->documentElement->firstChild;
23+
$target->before($doc->documentElement->lastChild);
24+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
25+
26+
$doc = new \DOMDocument();
27+
$doc->loadXML('<a>foo<last/></a>');
28+
$target = $doc->documentElement->lastChild;
29+
$target->before($doc->documentElement->firstChild);
30+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
31+
32+
33+
$doc = new \DOMDocument();
34+
$doc->loadXML('<a>foo<last/></a>');
35+
$target = $doc->documentElement->firstChild;
36+
$target->before($target, $doc->documentElement->lastChild);
37+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
38+
39+
$doc = new \DOMDocument();
40+
$doc->loadXML('<a>foo<last/></a>');
41+
$target = $doc->documentElement->firstChild;
42+
$target->before($doc->documentElement->lastChild, $target);
43+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
44+
45+
46+
$doc = new \DOMDocument();
47+
$doc->loadXML('<a>foo<last/></a>');
48+
$target = $doc->documentElement->lastChild;
49+
$target->before($target, $doc->documentElement->firstChild);
50+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
51+
52+
$doc = new \DOMDocument();
53+
$doc->loadXML('<a>foo<last/></a>');
54+
$target = $doc->documentElement->lastChild;
55+
$target->before($doc->documentElement->firstChild, $target);
56+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
57+
58+
59+
$doc = new \DOMDocument();
60+
$doc->loadXML('<a>foo<last/></a>');
61+
$target = $doc->documentElement->firstChild;
62+
$target->before('bar','baz');
63+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
64+
65+
$doc = new \DOMDocument();
66+
$doc->loadXML('<a>foo<last/></a>');
67+
$target = $doc->documentElement->lastChild;
68+
$target->before('bar','baz');
69+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
70+
71+
72+
$doc = new \DOMDocument();
73+
$doc->loadXML('<a>foo<last/></a>');
74+
$target = $doc->documentElement->firstChild;
75+
$target->before($target, 'bar','baz');
76+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
77+
78+
$doc = new \DOMDocument();
79+
$doc->loadXML('<a>foo<last/></a>');
80+
$target = $doc->documentElement->firstChild;
81+
$target->before('bar', $target, 'baz');
82+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
83+
84+
$doc = new \DOMDocument();
85+
$doc->loadXML('<a>foo<last/></a>');
86+
$target = $doc->documentElement->firstChild;
87+
$target->before('bar', 'baz', $target);
88+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
89+
90+
91+
92+
$doc = new \DOMDocument();
93+
$doc->loadXML('<a>foo<last/></a>');
94+
$target = $doc->documentElement->lastChild;
95+
$target->before($target, 'bar','baz');
96+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
97+
98+
$doc = new \DOMDocument();
99+
$doc->loadXML('<a>foo<last/></a>');
100+
$target = $doc->documentElement->lastChild;
101+
$target->before('bar', $target, 'baz');
102+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
103+
104+
$doc = new \DOMDocument();
105+
$doc->loadXML('<a>foo<last/></a>');
106+
$target = $doc->documentElement->lastChild;
107+
$target->before('bar', 'baz', $target);
108+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
109+
110+
111+
112+
$doc = new \DOMDocument();
113+
$doc->loadXML('<a>foo<last/></a>');
114+
$target = $doc->documentElement->firstChild;
115+
$target->before('bar', $target, $doc->documentElement->lastChild);
116+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
117+
118+
119+
$doc = new \DOMDocument();
120+
$doc->loadXML('<a>foo<last/></a>');
121+
$target = $doc->documentElement->firstChild;
122+
$target->before($target, 'bar', $doc->documentElement->lastChild);
123+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
124+
125+
126+
$doc = new \DOMDocument();
127+
$doc->loadXML('<a>foo<last/></a>');
128+
$target = $doc->documentElement->firstChild;
129+
$target->before($target, $doc->documentElement->lastChild, 'bar');
130+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
131+
132+
133+
134+
135+
$doc = new \DOMDocument();
136+
$doc->loadXML('<a>foo<last/></a>');
137+
$target = $doc->documentElement->lastChild;
138+
$target->before('bar', $doc->documentElement->firstChild, $target);
139+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
140+
141+
142+
$doc = new \DOMDocument();
143+
$doc->loadXML('<a>foo<last/></a>');
144+
$target = $doc->documentElement->lastChild;
145+
$target->before($doc->documentElement->firstChild, 'bar', $target);
146+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
147+
148+
149+
$doc = new \DOMDocument();
150+
$doc->loadXML('<a>foo<last/></a>');
151+
$target = $doc->documentElement->lastChild;
152+
$target->before($doc->documentElement->firstChild, $target, 'bar');
153+
echo $doc->saveXML($doc->documentElement).PHP_EOL;
154+
155+
?>
156+
--EXPECTF--
157+
<a>foo<last/></a>
158+
<a>foo<last/></a>
159+
<a><last/>foo</a>
160+
<a>foo<last/></a>
161+
<a>foo<last/></a>
162+
<a><last/>foo</a>
163+
<a><last/>foo</a>
164+
<a>foo<last/></a>
165+
<a>barbazfoo<last/></a>
166+
<a>foobarbaz<last/></a>
167+
<a>foobarbaz<last/></a>
168+
<a>barfoobaz<last/></a>
169+
<a>barbazfoo<last/></a>
170+
<a>foo<last/>barbaz</a>
171+
<a>foobar<last/>baz</a>
172+
<a>foobarbaz<last/></a>
173+
<a>barfoo<last/></a>
174+
<a>foobar<last/></a>
175+
<a>foo<last/>bar</a>
176+
<a>barfoo<last/></a>
177+
<a>foobar<last/></a>
178+
<a>foo<last/>bar</a>

0 commit comments

Comments
 (0)