From 5c1249391b41267a8a8b45caa68aaddaf2b94621 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 8 Oct 2024 20:09:48 +0200 Subject: [PATCH] Fix GH-16292: Segmentation fault in ext/xmlreader/php_xmlreader.c:1282 3 issues: 1) RETURN_NULL() was used via the macro NODE_GET_OBJ(), but the function returns false on failure and cannot return null according to its stub. 2) The struct layout of the different implementors of libxml only guarantees overlap between the node pointer and the document reference, so accessing the std zend_object may not work. 3) DOC_GET_OBJ() wasn't using ZSTR_VAL(). Closes GH-16307. --- NEWS | 3 +++ ext/dom/xml_common.h | 18 ++++++++++-------- ext/xmlreader/php_xmlreader.c | 8 +++++++- ext/xmlreader/tests/gh16292.phpt | 26 ++++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 ext/xmlreader/tests/gh16292.phpt diff --git a/NEWS b/NEWS index b4ecf799a3721..ac01709f717ee 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.2.26 +- XMLReader: + . Fixed bug GH-16292 (Segmentation fault in ext/xmlreader/php_xmlreader.c). + (nielsdos) 24 Oct 2024, PHP 8.2.25 diff --git a/ext/dom/xml_common.h b/ext/dom/xml_common.h index da210aae96707..b0a41398b6be9 100644 --- a/ext/dom/xml_common.h +++ b/ext/dom/xml_common.h @@ -60,21 +60,23 @@ PHP_DOM_EXPORT xmlNodePtr dom_object_get_node(dom_object *obj); (const xmlChar *) "http://www.w3.org/2000/xmlns/" #define NODE_GET_OBJ(__ptr, __id, __prtype, __intern) { \ - __intern = Z_LIBXML_NODE_P(__id); \ + zval *__zv = (__id); \ + __intern = Z_LIBXML_NODE_P(__zv); \ if (__intern->node == NULL || !(__ptr = (__prtype)__intern->node->node)) { \ - php_error_docref(NULL, E_WARNING, "Couldn't fetch %s", \ - ZSTR_VAL(__intern->std.ce->name));\ - RETURN_NULL();\ + php_error_docref(NULL, E_WARNING, "Couldn't fetch %s", \ + ZSTR_VAL(Z_OBJCE_P(__zv)->name));\ + RETURN_NULL();\ } \ } #define DOC_GET_OBJ(__ptr, __id, __prtype, __intern) { \ - __intern = Z_LIBXML_NODE_P(__id); \ + zval *__zv = (__id); \ + __intern = Z_LIBXML_NODE_P(__zv); \ if (__intern->document != NULL) { \ if (!(__ptr = (__prtype)__intern->document->ptr)) { \ - php_error_docref(NULL, E_WARNING, "Couldn't fetch %s", __intern->std.ce->name);\ - RETURN_NULL();\ - } \ + php_error_docref(NULL, E_WARNING, "Couldn't fetch %s", ZSTR_VAL(Z_OBJCE_P(__zv)->name));\ + RETURN_NULL();\ + } \ } \ } diff --git a/ext/xmlreader/php_xmlreader.c b/ext/xmlreader/php_xmlreader.c index 1b4bc6bcef4b1..e3ca41399355d 100644 --- a/ext/xmlreader/php_xmlreader.c +++ b/ext/xmlreader/php_xmlreader.c @@ -1122,7 +1122,13 @@ PHP_METHOD(XMLReader, expand) } if (basenode != NULL) { - NODE_GET_OBJ(node, basenode, xmlNodePtr, domobj); + /* Note: cannot use NODE_GET_OBJ here because of the wrong return type */ + domobj = Z_LIBXML_NODE_P(basenode); + if (UNEXPECTED(domobj->node == NULL || domobj->node->node == NULL)) { + php_error_docref(NULL, E_WARNING, "Couldn't fetch %s", ZSTR_VAL(Z_OBJCE_P(basenode)->name)); + RETURN_FALSE; + } + node = domobj->node->node; docp = node->doc; } diff --git a/ext/xmlreader/tests/gh16292.phpt b/ext/xmlreader/tests/gh16292.phpt new file mode 100644 index 0000000000000..f3cd37e951d5f --- /dev/null +++ b/ext/xmlreader/tests/gh16292.phpt @@ -0,0 +1,26 @@ +--TEST-- +GH-16292 (Segmentation fault in ext/xmlreader/php_xmlreader.c:1282) +--EXTENSIONS-- +dom +xmlreader +--FILE-- + +new book'; +$reader = new XMLReader(); +$reader->XML($xmlstring); +while ($reader->read()) { + if ($reader->localName == "book") { + var_dump($reader->expand($character_data)); + } +} + +?> +--EXPECTF-- +Warning: XMLReader::expand(): Couldn't fetch DOMCharacterData in %s on line %d +bool(false) + +Warning: XMLReader::expand(): Couldn't fetch DOMCharacterData in %s on line %d +bool(false)