From a6d17bffe115a0bbed8a3260ef588124e20973f8 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sun, 17 Dec 2023 01:35:15 +0100 Subject: [PATCH 1/3] Fix GH-12962: Double free of init_file in phpdbg_prompt.c See GH-12962 for analysis. Closes GH-12963. --- NEWS | 3 +++ sapi/phpdbg/phpdbg_prompt.c | 2 +- sapi/phpdbg/tests/gh12962.phpt | 13 +++++++++++++ sapi/phpdbg/tests/gh12962/.phpdbginit | 2 ++ 4 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 sapi/phpdbg/tests/gh12962.phpt create mode 100644 sapi/phpdbg/tests/gh12962/.phpdbginit diff --git a/NEWS b/NEWS index 4a53cc18122..f86e393a461 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,9 @@ PHP NEWS . Added workaround for SELinux mprotect execheap issue. See https://bugzilla.kernel.org/show_bug.cgi?id=218258. (ilutov) +- PHPDBG: + . Fixed bug GH-12962 (Double free of init_file in phpdbg_prompt.c). (nielsdos) + 21 Dec 2023, PHP 8.2.14 - Core: diff --git a/sapi/phpdbg/phpdbg_prompt.c b/sapi/phpdbg/phpdbg_prompt.c index ffc40cb0c96..994ac829b0a 100644 --- a/sapi/phpdbg/phpdbg_prompt.c +++ b/sapi/phpdbg/phpdbg_prompt.c @@ -364,7 +364,7 @@ void phpdbg_init(char *init_file, size_t init_file_len, bool use_default) /* {{{ } ZEND_IGNORE_VALUE(asprintf(&init_file, "%s/%s", scan_dir, PHPDBG_INIT_FILENAME)); - phpdbg_try_file_init(init_file, strlen(init_file), 1); + phpdbg_try_file_init(init_file, strlen(init_file), 0); free(init_file); if (i == -1) { break; diff --git a/sapi/phpdbg/tests/gh12962.phpt b/sapi/phpdbg/tests/gh12962.phpt new file mode 100644 index 00000000000..c5cf9425d7c --- /dev/null +++ b/sapi/phpdbg/tests/gh12962.phpt @@ -0,0 +1,13 @@ +--TEST-- +GH-12962 (Double free of init_file in phpdbg_prompt.c) +--SKIPIF-- + +--FILE-- + +--EXPECT-- +Executed .phpdbginit diff --git a/sapi/phpdbg/tests/gh12962/.phpdbginit b/sapi/phpdbg/tests/gh12962/.phpdbginit new file mode 100644 index 00000000000..29184ddf7c8 --- /dev/null +++ b/sapi/phpdbg/tests/gh12962/.phpdbginit @@ -0,0 +1,2 @@ +ev "Executed .phpdbginit" +q From abf4c116b18a5d0dbe71ee9c8788b065a74bf1c0 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 12 Dec 2023 20:33:23 +0100 Subject: [PATCH 2/3] Fix getting the address of an uninitialized property of a SimpleXMLElement resulting in a crash Closes GH-12945. --- NEWS | 4 ++++ ext/simplexml/simplexml.c | 3 +++ .../tests/get_prop_address_not_initialized.phpt | 17 +++++++++++++++++ 3 files changed, 24 insertions(+) create mode 100644 ext/simplexml/tests/get_prop_address_not_initialized.phpt diff --git a/NEWS b/NEWS index f86e393a461..a061e779884 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,10 @@ PHP NEWS - PHPDBG: . Fixed bug GH-12962 (Double free of init_file in phpdbg_prompt.c). (nielsdos) +- SimpleXML: + . Fix getting the address of an uninitialized property of a SimpleXMLElement + resulting in a crash. (nielsdos) + 21 Dec 2023, PHP 8.2.14 - Core: diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 37c6aa8fc0a..ce7c96a0d46 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -636,6 +636,9 @@ static zval *sxe_property_get_adr(zend_object *object, zend_string *zname, int f sxe = php_sxe_fetch_object(object); GET_NODE(sxe, node); + if (UNEXPECTED(!node)) { + return &EG(error_zval); + } name = ZSTR_VAL(zname); node = sxe_get_element_by_name(sxe, node, &name, &type); if (node) { diff --git a/ext/simplexml/tests/get_prop_address_not_initialized.phpt b/ext/simplexml/tests/get_prop_address_not_initialized.phpt new file mode 100644 index 00000000000..e6f07405b9b --- /dev/null +++ b/ext/simplexml/tests/get_prop_address_not_initialized.phpt @@ -0,0 +1,17 @@ +--TEST-- +Getting the address of an uninitialized property of a SimpleXMLElement +--EXTENSIONS-- +simplexml +--FILE-- +newInstanceWithoutConstructor(); +$sxe->a['b'] = 'b'; + +?> +--EXPECTF-- +Fatal error: Uncaught Error: SimpleXMLElement is not properly initialized in %s:%d +Stack trace: +#0 {main} + thrown in %s on line %d From f75931ad9e5649d43c6f7cb722abf485bb9784d3 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 12 Dec 2023 21:05:58 +0100 Subject: [PATCH 3/3] Fix GH-12929: SimpleXMLElement with stream_wrapper_register can segfault Move SimpleXML invalidation code after node checks This is safe, i.e. the tree hasn't been modified yet, because either we didn't call a libxml modification function yet, or xmlNewChild is called with a NULL pointer, which makes it bail out and return NULL. Closes GH-12947. --- NEWS | 2 ++ ext/simplexml/simplexml.c | 12 ++++++------ ext/simplexml/tests/gh12929.phpt | 29 +++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 ext/simplexml/tests/gh12929.phpt diff --git a/NEWS b/NEWS index 69d3ffb52a4..8d6fc8d5444 100644 --- a/NEWS +++ b/NEWS @@ -34,6 +34,8 @@ PHP NEWS - SimpleXML: . Fix getting the address of an uninitialized property of a SimpleXMLElement resulting in a crash. (nielsdos) + . Fixed bug GH-12929 (SimpleXMLElement with stream_wrapper_register can + segfault). (nielsdos) 07 Dec 2023, PHP 8.3.1RC1 diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index ae3c98b7e3e..eebf4af0239 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -421,8 +421,6 @@ long_dim: GET_NODE(sxe, node); - php_libxml_invalidate_node_list_cache_from_doc(node->doc); - if (sxe->iter.type == SXE_ITER_ATTRLIST) { attribs = 1; elements = 0; @@ -483,6 +481,8 @@ long_dim: } if (node) { + php_libxml_invalidate_node_list_cache_from_doc(node->doc); + if (attribs) { if (Z_TYPE_P(member) == IS_LONG) { while (attr && nodendx <= Z_LVAL_P(member)) { @@ -797,8 +797,6 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements GET_NODE(sxe, node); - php_libxml_invalidate_node_list_cache_from_doc(node->doc); - if (Z_TYPE_P(member) == IS_LONG) { if (sxe->iter.type != SXE_ITER_ATTRLIST) { attribs = 0; @@ -822,6 +820,8 @@ static void sxe_prop_dim_delete(zend_object *object, zval *member, bool elements } if (node) { + php_libxml_invalidate_node_list_cache_from_doc(node->doc); + if (attribs) { if (Z_TYPE_P(member) == IS_LONG) { int nodendx = 0; @@ -1678,8 +1678,6 @@ PHP_METHOD(SimpleXMLElement, addChild) sxe = Z_SXEOBJ_P(ZEND_THIS); GET_NODE(sxe, node); - php_libxml_invalidate_node_list_cache_from_doc(node->doc); - if (sxe->iter.type == SXE_ITER_ATTRLIST) { php_error_docref(NULL, E_WARNING, "Cannot add element to attributes"); return; @@ -1692,6 +1690,8 @@ PHP_METHOD(SimpleXMLElement, addChild) return; } + php_libxml_invalidate_node_list_cache_from_doc(node->doc); + localname = xmlSplitQName2((xmlChar *)qname, &prefix); if (localname == NULL) { localname = xmlStrdup((xmlChar *)qname); diff --git a/ext/simplexml/tests/gh12929.phpt b/ext/simplexml/tests/gh12929.phpt new file mode 100644 index 00000000000..2ae89346dba --- /dev/null +++ b/ext/simplexml/tests/gh12929.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-12929 (SimpleXMLElement with stream_wrapper_register can segfault) +--EXTENSIONS-- +simplexml +--FILE-- +getMessage(), "\n"; + echo $e->getPrevious()->getMessage(), "\n"; +} + +$scheme = "foo2"; +stream_wrapper_register($scheme, "SimpleXMLElement"); +try { + file_get_contents($scheme . "://x"); +} catch (Error $e) { + echo $e->getMessage(), "\n"; + echo $e->getPrevious()->getMessage(), "\n"; +} +?> +--EXPECT-- +It's not possible to assign a complex type to properties, resource given +SimpleXMLElement is not properly initialized +It's not possible to assign a complex type to properties, resource given +SimpleXMLElement is not properly initialized