Skip to content

Commit

Permalink
Fix phpGH-17409: Assertion failure Zend/zend_hash.c:1730
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nielsdos committed Jan 9, 2025
1 parent 783ecad commit e2d3a99
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 2 deletions.
4 changes: 2 additions & 2 deletions ext/simplexml/simplexml.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
/* }}} */
Expand Down
22 changes: 22 additions & 0 deletions ext/simplexml/tests/gh17409.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
--TEST--
GH-17409 (Assertion failure Zend/zend_hash.c)
--EXTENSIONS--
simplexml
--CREDITS--
YuanchengJiang
--FILE--
<?php
$root = simplexml_load_string('<?xml version="1.0"?>
<root xmlns:reserved="reserved-ns">
<child reserved:attribute="Sample" />
</root>
');
// Need to use $GLOBALS such that simplexml object is destroyed
var_dump(array_merge_recursive($GLOBALS, $GLOBALS)["root"]);
?>
--EXPECT--
array(1) {
["child"]=>
array(0) {
}
}

0 comments on commit e2d3a99

Please sign in to comment.