From fbe772c978537cf56751d7c9f72ba1824d96fa82 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 9 Aug 2026 09:54:17 -0400 Subject: [PATCH] Reject NUL bytes in the SimpleXMLElement constructor 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 --- NEWS | 2 ++ UPGRADING | 8 +++++ ext/simplexml/simplexml.c | 9 +++--- ext/simplexml/tests/sxe_ctor_nul_path.phpt | 37 ++++++++++++++++++++++ 4 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 ext/simplexml/tests/sxe_ctor_nul_path.phpt 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-- +'); +$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("\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