From b282dd749f8b4133724160e3751e2edda03593ae Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 28 Jul 2024 20:15:53 +0200 Subject: [PATCH] Fix UAF when removing doctype and using foreach iteration This is an old bug, but this is pretty easy to fix. It's basically applying the same fix as I did for e878b9f. Reported by YuanchengJiang. Closes GH-15143. --- NEWS | 3 +++ ext/dom/dom_iterators.c | 2 +- ext/dom/nodelist.c | 2 +- ext/dom/php_dom.h | 1 + ext/dom/tests/uaf_doctype_iterator.phpt | 26 +++++++++++++++++++++++++ 5 files changed, 32 insertions(+), 2 deletions(-) create mode 100644 ext/dom/tests/uaf_doctype_iterator.phpt diff --git a/NEWS b/NEWS index 43bb8f54a8e73..0036ce3230afd 100644 --- a/NEWS +++ b/NEWS @@ -16,6 +16,9 @@ PHP NEWS . Fixed case when curl_error returns an empty string. (David Carlier) +- DOM: + . Fix UAF when removing doctype and using foreach iteration. (nielsdos) + - FFI: . Fixed bug GH-14286 (ffi enum type (when enum has no name) make memory leak). (nielsdos, dstogov) diff --git a/ext/dom/dom_iterators.c b/ext/dom/dom_iterators.c index 670f08a679f2e..866cb5a07e99f 100644 --- a/ext/dom/dom_iterators.c +++ b/ext/dom/dom_iterators.c @@ -295,7 +295,7 @@ zend_object_iterator *php_dom_get_iterator(zend_class_entry *ce, zval *object, i if (objmap->nodetype == XML_ATTRIBUTE_NODE) { curnode = (xmlNodePtr) nodep->properties; } else { - curnode = (xmlNodePtr) nodep->children; + curnode = dom_nodelist_iter_start_first_child(nodep); } } else { if (nodep->type == XML_DOCUMENT_NODE || nodep->type == XML_HTML_DOCUMENT_NODE) { diff --git a/ext/dom/nodelist.c b/ext/dom/nodelist.c index a8e713f67d03e..6ed561e6d68e0 100644 --- a/ext/dom/nodelist.c +++ b/ext/dom/nodelist.c @@ -31,7 +31,7 @@ * Since: */ -static xmlNodePtr dom_nodelist_iter_start_first_child(xmlNodePtr nodep) +xmlNodePtr dom_nodelist_iter_start_first_child(xmlNodePtr nodep) { if (nodep->type == XML_ENTITY_REF_NODE) { /* See entityreference.c */ diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index ccf665c417cd8..d0637e4674e3d 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -156,6 +156,7 @@ void php_dom_named_node_map_get_item_into_zval(dom_nnodemap_object *objmap, zend void php_dom_nodelist_get_item_into_zval(dom_nnodemap_object *objmap, zend_long index, zval *return_value); int php_dom_get_namednodemap_length(dom_object *obj); int php_dom_get_nodelist_length(dom_object *obj); +xmlNodePtr dom_nodelist_iter_start_first_child(xmlNodePtr nodep); #define DOM_GET_OBJ(__ptr, __id, __prtype, __intern) { \ __intern = Z_DOMOBJ_P(__id); \ diff --git a/ext/dom/tests/uaf_doctype_iterator.phpt b/ext/dom/tests/uaf_doctype_iterator.phpt new file mode 100644 index 0000000000000..b166a83955248 --- /dev/null +++ b/ext/dom/tests/uaf_doctype_iterator.phpt @@ -0,0 +1,26 @@ +--TEST-- +UAF when removing doctype and iterating over the child nodes +--EXTENSIONS-- +dom +--CREDITS-- +Yuancheng Jiang +--FILE-- +loadXML(<< +]> +&foo1; +XML); +$ref = $dom->documentElement->firstChild; +$nodes = $ref->childNodes; +$dom->removeChild($dom->doctype); +foreach($nodes as $str) {} +var_dump($nodes); +?> +--EXPECTF-- +object(DOMNodeList)#%d (1) { + ["length"]=> + int(0) +}