[php-src] master: Reject NUL bytes in the SimpleXMLElement constructor (#23069)

Author: Ilia Alshanetsky (iliaal)
Committer: GitHub (web-flow)
Pusher: iliaal
Date: 2026-08-09T19:44:01-04:00

Commit: https://github.com/php/php-src/commit/bda5a4943ec785f9cb2453da32efa0ece34afa53
Raw diff: https://github.com/php/php-src/commit/bda5a4943ec785f9cb2453da32efa0ece34afa53.diff

Reject NUL bytes in the SimpleXMLElement constructor (#23069)

With dataIsURL set the first argument goes to xmlReadFile as a C string,
so new SimpleXMLElement("/tmp/ok.xml\0evil", 0, true) truncates at the NUL
and quietly loads /tmp/ok.xml. simplexml_load_file() and
SimpleXMLElement::asXML() already declare their path argument as a path;
the constructor took a plain string, so the check never ran.

Closes GH-23069

Changed paths:
  A  ext/simplexml/tests/sxe_ctor_nul_path.phpt
  M  NEWS
  M  UPGRADING
  M  ext/simplexml/simplexml.c


Diff:

diff --git a/NEWS b/NEWS
index d7b60866a324..39d5a948e26f 100644
--- a/NEWS
+++ b/NEWS
@@ -70,6 +70,8 @@ PHP                                                                        NEWS
 - SimpleXML:
   . Fixed integer element offsets that cannot resolve aliasing an existing
     element. (iliaal)
+  . SimpleXMLElement::__construct() now raises a ValueError when the $data
+    argument contains NUL bytes. (iliaal)
 
 - Standard:
   . Added the "filter.max_filter_count" stream context option for php://filter
diff --git a/UPGRADING b/UPGRADING
index ba4bf182911f..e4669740b35f 100644
--- a/UPGRADING
+++ b/UPGRADING
@@ -200,6 +200,14 @@ PHP 8.6 UPGRADE NOTES
     SplFileObject::seek() past EOF now produces the same key() value as
     SplTempFileObject; the two previously returned different values.
 
+- SimpleXML:
+  . SimpleXMLElement::__construct() now raises a ValueError when the $data
+    argument contains NUL bytes, matching simplexml_load_file(). With
+    $dataIsURL set it previously truncated the path at the first NUL byte.
+    Without it the string went to libxml, which at default options rejects a
+    NUL on current versions but accepts the truncated document on older ones
+    and under LIBXML_RECOVER.
+
 - Standard:
   . array_intersect() with at least two arrays now converts values to strings
     while scanning its inputs instead of during sort comparisons. This can
diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c
index 1058c463cc60..828262f18aad 100644
--- a/ext/simplexml/simplexml.c
+++ b/ext/simplexml/simplexml.c
@@ -2283,18 +2283,17 @@ PHP_FUNCTION(simplexml_load_string)
 PHP_METHOD(SimpleXMLElement, __construct)
 {
 	php_sxe_object *sxe = Z_SXEOBJ_P(ZEND_THIS);
-	char           *data;
+	zend_string    *data;
 	zend_string    *ns = zend_empty_string;
-	size_t             data_len;
 	xmlDocPtr       docp;
 	zend_long            options = 0;
 	bool       is_url = false, isprefix = false;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|lbSb", &data, &data_len, &options, &is_url, &ns, &isprefix) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "P|lbSb", &data, &options, &is_url, &ns, &isprefix) == FAILURE) {
 		RETURN_THROWS();
 	}
 
-	if (ZEND_SIZE_T_INT_OVFL(data_len)) {
+	if (ZEND_SIZE_T_INT_OVFL(ZSTR_LEN(data))) {
 		zend_argument_error(zend_ce_exception, 1, "is too long");
 		RETURN_THROWS();
 	}
@@ -2308,7 +2307,7 @@ PHP_METHOD(SimpleXMLElement, __construct)
 	}
 
 	PHP_LIBXML_SANITIZE_GLOBALS(read_file_or_memory);
-	docp = is_url ? xmlReadFile(data, NULL, (int)options) : xmlReadMemory(data, (int)data_len, NULL, NULL, (int)options);
+	docp = is_url ? xmlReadFile(ZSTR_VAL(data), NULL, (int)options) : xmlReadMemory(ZSTR_VAL(data), (int)ZSTR_LEN(data), NULL, NULL, (int)options);
 	PHP_LIBXML_RESTORE_GLOBALS(read_file_or_memory);
 
 	if (!docp) {
diff --git a/ext/simplexml/tests/sxe_ctor_nul_path.phpt b/ext/simplexml/tests/sxe_ctor_nul_path.phpt
new file mode 100644
index 000000000000..ac451747a4b9
--- /dev/null
+++ b/ext/simplexml/tests/sxe_ctor_nul_path.phpt
@@ -0,0 +1,37 @@
+--TEST--
+SimpleXMLElement constructor rejects NUL bytes in $data
+--EXTENSIONS--
+simplexml
+--FILE--
+<?php
+$tmp = tempnam(sys_get_temp_dir(), 'sxe');
+file_put_contents($tmp, '<r/>');
+$path = $tmp . "\0evil";
+
+try {
+    new SimpleXMLElement($path, 0, true);
+    echo "url mode: loaded\n";
+} catch (Throwable $e) {
+    echo $e::class, ": ", $e->getMessage(), "\n";
+}
+
+try {
+    new SimpleXMLElement("<r/>\0evil");
+    echo "data mode: loaded\n";
+} catch (Throwable $e) {
+    echo $e::class, ": ", $e->getMessage(), "\n";
+}
+
+try {
+    simplexml_load_file($path);
+    echo "load_file: loaded\n";
+} catch (Throwable $e) {
+    echo $e::class, ": ", $e->getMessage(), "\n";
+}
+
+unlink($tmp);
+?>
+--EXPECT--
+ValueError: SimpleXMLElement::__construct(): Argument #1 ($data) must not contain any null bytes
+ValueError: SimpleXMLElement::__construct(): Argument #1 ($data) must not contain any null bytes
+ValueError: simplexml_load_file(): Argument #1 ($filename) must not contain any null bytes