From a2a7287b870cc21154905ff8a700cab2d7445a3f Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Thu, 9 Jan 2025 19:15:55 +0100 Subject: [PATCH] Fix GH-17409: Assertion failure Zend/zend_hash.c:1730 The array merging function may still hold the properties array while the object is already being destroyed. Therefore, we should take into account the refcount in simplexml's destruction code. It may be possible to trigger this in other ways too. Closes GH-17421. --- NEWS | 3 +++ ext/simplexml/simplexml.c | 4 ++-- ext/simplexml/tests/gh17409.phpt | 22 ++++++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) create mode 100644 ext/simplexml/tests/gh17409.phpt diff --git a/NEWS b/NEWS index 125c704b122..b1319961dc0 100644 --- a/NEWS +++ b/NEWS @@ -28,6 +28,9 @@ PHP NEWS - PHPDBG: . Fix crashes in function registration + test. (nielsdos, Girgias) +- SimpleXML: + . Fixed bug GH-17409 (Assertion failure Zend/zend_hash.c:1730). (nielsdos) + - SNMP: . Fixed bug GH-17330 (SNMP::setSecurity segfault on closed session). (David Carlier) diff --git a/ext/simplexml/simplexml.c b/ext/simplexml/simplexml.c index 00551267a45..18bfa31271e 100644 --- a/ext/simplexml/simplexml.c +++ b/ext/simplexml/simplexml.c @@ -2189,8 +2189,8 @@ static void sxe_object_free_storage(zend_object *object) sxe_object_free_iterxpath(sxe); if (sxe->properties) { - zend_hash_destroy(sxe->properties); - FREE_HASHTABLE(sxe->properties); + ZEND_ASSERT(!(GC_FLAGS(sxe->properties) & IS_ARRAY_IMMUTABLE)); + zend_hash_release(sxe->properties); } } /* }}} */ diff --git a/ext/simplexml/tests/gh17409.phpt b/ext/simplexml/tests/gh17409.phpt new file mode 100644 index 00000000000..f91ef38ba70 --- /dev/null +++ b/ext/simplexml/tests/gh17409.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-17409 (Assertion failure Zend/zend_hash.c) +--EXTENSIONS-- +simplexml +--CREDITS-- +YuanchengJiang +--FILE-- + + + + +'); +// Need to use $GLOBALS such that simplexml object is destroyed +var_dump(array_merge_recursive($GLOBALS, $GLOBALS)["root"]); +?> +--EXPECT-- +array(1) { + ["child"]=> + array(0) { + } +}