Skip to content

Commit 1a71661

Browse files
iliaaliliaal
committed
Fix SimpleXML integer offsets that cannot resolve aliasing a node
sxe_get_element_by_offset scanned with nodendx <= offset, so a negative offset skipped the loop and returned the node it started from, and the SXE_ITER_NONE branches of the read and write handlers aliased the node for every offset other than 0. Reads and isset() reported an existing element and writes overwrote it. An offset that resolves to no element now warns and leaves the document alone. Closes GH-23068
1 parent 9366c61 commit 1a71661

3 files changed

Lines changed: 121 additions & 12 deletions

File tree

‎NEWS‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ PHP NEWS
4242
. Fixed bug GH-23043 (broken session id code can cause zend_mm_heap
4343
corrupted). (ndossche)
4444

45+
- SimpleXML:
46+
. Fixed integer element offsets that cannot resolve aliasing an existing
47+
element. (iliaal)
48+
4549
- Sockets:
4650
. Fixed various memory related issues in ext/sockets. (David Carlier)
4751

‎ext/simplexml/simplexml.c‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ static xmlNodePtr sxe_get_element_by_offset(php_sxe_object *sxe, zend_long offse
134134
return NULL;
135135
}
136136
}
137-
while (node && nodendx <= offset) {
137+
while (node && (offset < 0 || nodendx <= offset)) {
138138
if (node->type == XML_ELEMENT_NODE && match_ns(node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
139139
if (sxe->iter.type == SXE_ITER_CHILD || (
140140
sxe->iter.type == SXE_ITER_ELEMENT && xmlStrEqual(node->name, BAD_CAST ZSTR_VAL(sxe->iter.name)))) {
@@ -302,14 +302,16 @@ static zval *sxe_prop_dim_read(zend_object *object, zval *member, bool elements,
302302
}
303303
if (!member || Z_TYPE_P(member) == IS_LONG) {
304304
zend_long cnt = 0;
305+
bool appendable = true;
305306
xmlNodePtr mynode = node;
306307

307308
if (sxe->iter.type == SXE_ITER_CHILD) {
308309
node = php_sxe_get_first_node_non_destructive(sxe, node);
309310
}
310311
if (sxe->iter.type == SXE_ITER_NONE) {
311-
if (member && Z_LVAL_P(member) > 0) {
312-
php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only 0 such elements exist", mynode->name, Z_LVAL_P(member));
312+
if (member && Z_LVAL_P(member) != 0) {
313+
node = NULL;
314+
appendable = false;
313315
}
314316
} else if (member) {
315317
node = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt);
@@ -319,11 +321,13 @@ static zval *sxe_prop_dim_read(zend_object *object, zval *member, bool elements,
319321
if (node) {
320322
node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
321323
} else if (type == BP_VAR_W || type == BP_VAR_RW) {
322-
if (member && cnt < Z_LVAL_P(member)) {
324+
if (member && (!appendable || Z_LVAL_P(member) < 0 || cnt < Z_LVAL_P(member))) {
323325
php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only " ZEND_LONG_FMT " such elements exist", mynode->name, Z_LVAL_P(member), cnt);
324326
}
325-
node = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, NULL);
326-
node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
327+
if (appendable && (!member || Z_LVAL_P(member) >= 0)) {
328+
node = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, NULL);
329+
node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
330+
}
327331
}
328332
} else {
329333
/* In BP_VAR_IS mode only return a proper node if it actually exists. */
@@ -534,12 +538,15 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
534538
}
535539

536540
if (sxe->iter.type == SXE_ITER_NONE) {
537-
newnode = node;
538-
++counter;
539-
if (member && Z_LVAL_P(member) > 0) {
541+
if (member && Z_LVAL_P(member) != 0) {
540542
php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only 0 such elements exist", mynode->name, Z_LVAL_P(member));
541-
value = &EG(error_zval);
543+
if (value_str) {
544+
zend_string_release(value_str);
545+
}
546+
return &EG(error_zval);
542547
}
548+
newnode = node;
549+
++counter;
543550
} else if (member) {
544551
newnode = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt);
545552
if (newnode) {
@@ -586,10 +593,14 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
586593
newnode = xmlNewTextChild(mynode, NULL, (xmlChar *)Z_STRVAL_P(member), value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL);
587594
}
588595
} else if (!member || Z_TYPE_P(member) == IS_LONG) {
589-
if (member && cnt < Z_LVAL_P(member)) {
596+
if (member && (Z_LVAL_P(member) < 0 || cnt < Z_LVAL_P(member))) {
590597
php_error_docref(NULL, E_WARNING, "Cannot add element %s number " ZEND_LONG_FMT " when only " ZEND_LONG_FMT " such elements exist", mynode->name, Z_LVAL_P(member), cnt);
591598
}
592-
newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL);
599+
if (member && Z_LVAL_P(member) < 0) {
600+
value = &EG(error_zval);
601+
} else {
602+
newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL);
603+
}
593604
}
594605
} else if (attribs) {
595606
if (Z_TYPE_P(member) == IS_LONG) {

‎ext/simplexml/tests/sxe_int_offset_no_alias.phpt‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
--TEST--
2+
Integer offsets that cannot resolve must never alias or mutate a node
3+
--EXTENSIONS--
4+
simplexml
5+
--FILE--
6+
<?php
7+
function fresh(): SimpleXMLElement {
8+
return simplexml_load_string('<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>');
9+
}
10+
11+
function state(SimpleXMLElement $x): string {
12+
return trim(strstr($x->asXML(), '<r'));
13+
}
14+
15+
echo "== element list ==\n";
16+
$x = fresh();
17+
var_dump(isset($x->item[-1]));
18+
var_dump($x->item[-1]);
19+
var_dump((string) $x->item[0]);
20+
$x->item[-1] = 'Z';
21+
echo state($x), "\n";
22+
unset($x->item[-1]);
23+
echo state($x), "\n";
24+
$x->item[5] = 'P';
25+
echo state($x), "\n";
26+
27+
echo "== single element ==\n";
28+
$x = fresh();
29+
$n = $x->item[0];
30+
var_dump(isset($n[-1]));
31+
var_dump($n[-1]);
32+
var_dump($n[5]);
33+
$n[-1] = 'Z';
34+
echo state($x), "\n";
35+
$n[5] = 'Y';
36+
echo state($x), "\n";
37+
unset($n[-1]);
38+
echo state($x), "\n";
39+
var_dump((string) $n[0]);
40+
41+
echo "== nested write ==\n";
42+
$x = fresh();
43+
try {
44+
$x->item[-1]->kid = 'K';
45+
} catch (Throwable $e) {
46+
echo $e::class, ': ', $e->getMessage(), "\n";
47+
}
48+
echo state($x), "\n";
49+
50+
echo "== attributes ==\n";
51+
$x = fresh();
52+
$at = $x->attributes();
53+
var_dump(isset($at[-1]));
54+
var_dump($at[-1]);
55+
$at[-1] = 'z';
56+
echo state($x), "\n";
57+
?>
58+
--EXPECTF--
59+
== element list ==
60+
bool(false)
61+
NULL
62+
string(1) "a"
63+
64+
Warning: main(): Cannot add element item number -1 when only 3 such elements exist in %s on line %d
65+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>
66+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>
67+
68+
Warning: main(): Cannot add element item number 5 when only 3 such elements exist in %s on line %d
69+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item><item>P</item></r>
70+
== single element ==
71+
bool(false)
72+
NULL
73+
NULL
74+
75+
Warning: main(): Cannot add element item number -1 when only 0 such elements exist in %s on line %d
76+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>
77+
78+
Warning: main(): Cannot add element item number 5 when only 0 such elements exist in %s on line %d
79+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>
80+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>
81+
string(1) "a"
82+
== nested write ==
83+
84+
Warning: main(): Cannot add element item number -1 when only 3 such elements exist in %s on line %d
85+
86+
Notice: Indirect modification of overloaded element of SimpleXMLElement has no effect in %s on line %d
87+
Error: Attempt to assign property "kid" on null
88+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>
89+
== attributes ==
90+
bool(false)
91+
NULL
92+
93+
Warning: main(): Cannot change attribute number -1 when only 0 attributes exist in %s on line %d
94+
<r a="1" b="2"><item>a</item><item>b</item><item>c</item></r>

0 commit comments

Comments
 (0)