diff --git a/NEWS b/NEWS
index 474db936ef15..d6b609698de9 100644
--- a/NEWS
+++ b/NEWS
@@ -42,6 +42,10 @@ PHP NEWS
. Fixed bug GH-23043 (broken session id code can cause zend_mm_heap
corrupted). (ndossche)
+- SimpleXML:
+ . Fixed integer element offsets that cannot resolve aliasing an existing
+ element. (iliaal)
+
- Sockets:
. Fixed various memory related issues in ext/sockets. (David Carlier)
diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c
index 8cf8e657e58f..f3c1a073fcaf 100644
--- a/ext/simplexml/simplexml.c
+++ b/ext/simplexml/simplexml.c
@@ -134,7 +134,7 @@ static xmlNodePtr sxe_get_element_by_offset(php_sxe_object *sxe, zend_long offse
return NULL;
}
}
- while (node && nodendx <= offset) {
+ while (node && (offset < 0 || nodendx <= offset)) {
if (node->type == XML_ELEMENT_NODE && match_ns(node, sxe->iter.nsprefix, sxe->iter.isprefix)) {
if (sxe->iter.type == SXE_ITER_CHILD || (
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,
}
if (!member || Z_TYPE_P(member) == IS_LONG) {
zend_long cnt = 0;
+ bool appendable = true;
xmlNodePtr mynode = node;
if (sxe->iter.type == SXE_ITER_CHILD) {
node = php_sxe_get_first_node_non_destructive(sxe, node);
}
if (sxe->iter.type == SXE_ITER_NONE) {
- if (member && Z_LVAL_P(member) > 0) {
- 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));
+ if (member && Z_LVAL_P(member) != 0) {
+ node = NULL;
+ appendable = false;
}
} else if (member) {
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,
if (node) {
node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
} else if (type == BP_VAR_W || type == BP_VAR_RW) {
- if (member && cnt < Z_LVAL_P(member)) {
+ if (member && (Z_LVAL_P(member) < 0 || cnt < Z_LVAL_P(member))) {
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);
}
- node = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, NULL);
- node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
+ if (appendable && (!member || Z_LVAL_P(member) >= 0)) {
+ node = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, NULL);
+ node_as_zval(sxe, node, rv, SXE_ITER_NONE, NULL, sxe->iter.nsprefix, sxe->iter.isprefix);
+ }
}
} else {
/* In BP_VAR_IS mode only return a proper node if it actually exists. */
@@ -527,19 +531,18 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
if (!member || Z_TYPE_P(member) == IS_LONG) {
if (node->type == XML_ATTRIBUTE_NODE) {
zend_throw_error(NULL, "Cannot create duplicate attribute");
- if (value_str) {
- zend_string_release(value_str);
- }
- return &EG(error_zval);
+ value = &EG(error_zval);
+ goto out;
}
if (sxe->iter.type == SXE_ITER_NONE) {
- newnode = node;
- ++counter;
- if (member && Z_LVAL_P(member) > 0) {
+ if (member && Z_LVAL_P(member) != 0) {
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));
value = &EG(error_zval);
+ goto out;
}
+ newnode = node;
+ ++counter;
} else if (member) {
newnode = sxe_get_element_by_offset(sxe, Z_LVAL_P(member), node, &cnt);
if (newnode) {
@@ -586,10 +589,14 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
newnode = xmlNewTextChild(mynode, NULL, (xmlChar *)Z_STRVAL_P(member), value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL);
}
} else if (!member || Z_TYPE_P(member) == IS_LONG) {
- if (member && cnt < Z_LVAL_P(member)) {
+ if (member && (Z_LVAL_P(member) < 0 || cnt < Z_LVAL_P(member))) {
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);
}
- newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL);
+ if (member && Z_LVAL_P(member) < 0) {
+ value = &EG(error_zval);
+ } else {
+ newnode = xmlNewTextChild(mynode->parent, mynode->ns, mynode->name, value_str ? (xmlChar *)ZSTR_VAL(value_str) : NULL);
+ }
}
} else if (attribs) {
if (Z_TYPE_P(member) == IS_LONG) {
@@ -600,6 +607,7 @@ static zval *sxe_prop_dim_write(zend_object *object, zval *member, zval *value,
}
}
+out:
if (member == &tmp_zv) {
zval_ptr_dtor_str(&tmp_zv);
}
diff --git a/ext/simplexml/tests/sxe_int_offset_no_alias.phpt b/ext/simplexml/tests/sxe_int_offset_no_alias.phpt
new file mode 100644
index 000000000000..03e10e7a6c94
--- /dev/null
+++ b/ext/simplexml/tests/sxe_int_offset_no_alias.phpt
@@ -0,0 +1,94 @@
+--TEST--
+Integer offsets that cannot resolve must never alias or mutate a node
+--EXTENSIONS--
+simplexml
+--FILE--
+- a
- b
- c
');
+}
+
+function state(SimpleXMLElement $x): string {
+ return trim(strstr($x->asXML(), 'item[-1]));
+var_dump($x->item[-1]);
+var_dump((string) $x->item[0]);
+$x->item[-1] = 'Z';
+echo state($x), "\n";
+unset($x->item[-1]);
+echo state($x), "\n";
+$x->item[5] = 'P';
+echo state($x), "\n";
+
+echo "== single element ==\n";
+$x = fresh();
+$n = $x->item[0];
+var_dump(isset($n[-1]));
+var_dump($n[-1]);
+var_dump($n[5]);
+$n[-1] = 'Z';
+echo state($x), "\n";
+$n[5] = 'Y';
+echo state($x), "\n";
+unset($n[-1]);
+echo state($x), "\n";
+var_dump((string) $n[0]);
+
+echo "== nested write ==\n";
+$x = fresh();
+try {
+ $x->item[-1]->kid = 'K';
+} catch (Throwable $e) {
+ echo $e::class, ': ', $e->getMessage(), "\n";
+}
+echo state($x), "\n";
+
+echo "== attributes ==\n";
+$x = fresh();
+$at = $x->attributes();
+var_dump(isset($at[-1]));
+var_dump($at[-1]);
+$at[-1] = 'z';
+echo state($x), "\n";
+?>
+--EXPECTF--
+== element list ==
+bool(false)
+NULL
+string(1) "a"
+
+Warning: main(): Cannot add element item number -1 when only 3 such elements exist in %s on line %d
+- a
- b
- c
+- a
- b
- c
+
+Warning: main(): Cannot add element item number 5 when only 3 such elements exist in %s on line %d
+- a
- b
- c
- P
+== single element ==
+bool(false)
+NULL
+NULL
+
+Warning: main(): Cannot add element item number -1 when only 0 such elements exist in %s on line %d
+- a
- b
- c
+
+Warning: main(): Cannot add element item number 5 when only 0 such elements exist in %s on line %d
+- a
- b
- c
+- a
- b
- c
+string(1) "a"
+== nested write ==
+
+Warning: main(): Cannot add element item number -1 when only 3 such elements exist in %s on line %d
+
+Notice: Indirect modification of overloaded element of SimpleXMLElement has no effect in %s on line %d
+Error: Attempt to assign property "kid" on null
+- a
- b
- c
+== attributes ==
+bool(false)
+NULL
+
+Warning: main(): Cannot change attribute number -1 when only 0 attributes exist in %s on line %d
+- a
- b
- c