Commit 43a4f91c52a for php.net
commit 43a4f91c52ad01a1aa3a5ecbf89af12a42cbb377
Author: ndossche <7771979+ndossche@users.noreply.github.com>
Date: Sat Mar 14 11:30:59 2026 +0100
Fix concurrent iteration and deletion issues in SplObjectStorage
Closes GH-21443.
diff --git a/NEWS b/NEWS
index 7e1e34b8ae0..e05216ed331 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,8 @@ PHP NEWS
- SPL:
. Fixed bug GH-21499 (RecursiveArrayIterator getChildren UAF after parent
free). (Girgias)
+ . Fix concurrent iteration and deletion issues in SplObjectStorage.
+ (ndossche)
- Streams:
. Fixed bug GH-21468 (Segfault in file_get_contents w/ a https URL
diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c
index 622686dbd78..e653d1173b2 100644
--- a/ext/spl/spl_observer.c
+++ b/ext/spl/spl_observer.c
@@ -240,11 +240,25 @@ static zend_result spl_object_storage_detach(spl_SplObjectStorage *intern, zend_
return ret;
} /* }}}*/
+/* TODO: make this an official Zend API? */
+#define SPL_SAFE_HASH_FOREACH_PTR(_ht, _ptr) do { \
+ const HashTable *__ht = (_ht); \
+ zval *_z = __ht->arPacked; \
+ for (uint32_t _idx = 0; _idx < __ht->nNumUsed; _idx++, _z = ZEND_HASH_ELEMENT(__ht, _idx)) { \
+ if (UNEXPECTED(Z_ISUNDEF_P(_z))) continue; \
+ _ptr = Z_PTR_P(_z);
+
static void spl_object_storage_addall(spl_SplObjectStorage *intern, spl_SplObjectStorage *other) { /* {{{ */
spl_SplObjectStorageElement *element;
- ZEND_HASH_FOREACH_PTR(&other->storage, element) {
- spl_object_storage_attach(intern, element->obj, &element->inf);
+ SPL_SAFE_HASH_FOREACH_PTR(&other->storage, element) {
+ zval zv;
+ zend_object *obj = element->obj;
+ GC_ADDREF(obj);
+ ZVAL_COPY(&zv, &element->inf);
+ spl_object_storage_attach(intern, obj, &zv);
+ zval_ptr_dtor(&zv);
+ OBJ_RELEASE(obj);
} ZEND_HASH_FOREACH_END();
intern->index = 0;
@@ -626,10 +640,13 @@ PHP_METHOD(SplObjectStorage, removeAllExcept)
other = Z_SPLOBJSTORAGE_P(obj);
- ZEND_HASH_FOREACH_PTR(&intern->storage, element) {
- if (!spl_object_storage_contains(other, element->obj)) {
- spl_object_storage_detach(intern, element->obj);
+ SPL_SAFE_HASH_FOREACH_PTR(&intern->storage, element) {
+ zend_object *elem_obj = element->obj;
+ GC_ADDREF(elem_obj);
+ if (!spl_object_storage_contains(other, elem_obj)) {
+ spl_object_storage_detach(intern, elem_obj);
}
+ OBJ_RELEASE(elem_obj);
} ZEND_HASH_FOREACH_END();
zend_hash_internal_pointer_reset_ex(&intern->storage, &intern->pos);
diff --git a/ext/spl/tests/SplObjectStorage/concurrent_deletion.phpt b/ext/spl/tests/SplObjectStorage/concurrent_deletion.phpt
new file mode 100644
index 00000000000..ae063f1c9b0
--- /dev/null
+++ b/ext/spl/tests/SplObjectStorage/concurrent_deletion.phpt
@@ -0,0 +1,46 @@
+--TEST--
+SplObjectStorage: Concurrent deletion during iteration
+--CREDITS--
+cnitlrt
+--FILE--
+<?php
+class EvilStorage extends SplObjectStorage {
+ public function getHash($obj): string {
+ global $victim;
+ static $counter = 0;
+ if ($counter++ < 1024*2) {
+ // Re-entrant mutation during removeAllExcept iteration
+ for ($i = 0; $i < 5; $i++) {
+ $victim[new stdClass()] = null;
+ }
+ }
+ return spl_object_hash($obj);
+ }
+}
+
+$victim = new SplObjectStorage();
+$other = new EvilStorage();
+
+for ($i = 0; $i < 1024; $i++) {
+ $o = new stdClass();
+ $victim[$o] = null;
+ $other[$o] = null;
+}
+
+var_dump($victim->removeAllExcept($other));
+
+unset($victim, $other);
+$victim = new SplObjectStorage();
+$other = new EvilStorage();
+
+for ($i = 0; $i < 1024; $i++) {
+ $o = new stdClass();
+ $victim[$o] = null;
+ $other[$o] = null;
+}
+
+var_dump($other->addAll($victim));
+?>
+--EXPECTF--
+int(%d)
+int(1024)
diff --git a/ext/spl/tests/SplObjectStorage/concurrent_deletion_addall.phpt b/ext/spl/tests/SplObjectStorage/concurrent_deletion_addall.phpt
new file mode 100644
index 00000000000..af3fc381b56
--- /dev/null
+++ b/ext/spl/tests/SplObjectStorage/concurrent_deletion_addall.phpt
@@ -0,0 +1,43 @@
+--TEST--
+SplObjectStorage: Concurrent deletion during addAll
+--CREDITS--
+cnitlrt
+--FILE--
+<?php
+
+class EvilStorage extends SplObjectStorage {
+ public function getHash($obj): string {
+ global $storage;
+ unset($storage[$obj]);
+ return spl_object_hash($obj);
+ }
+}
+
+$storage = new SplObjectStorage();
+$storage[new stdClass] = 'foo';
+
+$evil = new EvilStorage();
+$evil->addAll($storage);
+
+var_dump($evil, $storage);
+
+?>
+--EXPECTF--
+object(EvilStorage)#%d (1) {
+ ["storage":"SplObjectStorage":private]=>
+ array(1) {
+ [0]=>
+ array(2) {
+ ["obj"]=>
+ object(stdClass)#%d (0) {
+ }
+ ["inf"]=>
+ string(3) "foo"
+ }
+ }
+}
+object(SplObjectStorage)#%d (1) {
+ ["storage":"SplObjectStorage":private]=>
+ array(0) {
+ }
+}
diff --git a/ext/spl/tests/SplObjectStorage/concurrent_deletion_removeexcept.phpt b/ext/spl/tests/SplObjectStorage/concurrent_deletion_removeexcept.phpt
new file mode 100644
index 00000000000..b2ed211b304
--- /dev/null
+++ b/ext/spl/tests/SplObjectStorage/concurrent_deletion_removeexcept.phpt
@@ -0,0 +1,35 @@
+--TEST--
+SplObjectStorage: Concurrent deletion during removeAllExcept
+--CREDITS--
+cnitlrt
+--FILE--
+<?php
+
+class EvilStorage extends SplObjectStorage {
+ public function getHash($obj): string {
+ global $storage;
+ unset($storage[$obj]);
+ return spl_object_hash($obj);
+ }
+}
+
+$storage = new SplObjectStorage();
+$storage[new stdClass] = 'foo';
+
+$evil = new EvilStorage();
+$storage->removeAllExcept($evil);
+
+var_dump($evil, $storage);
+
+?>
+--EXPECTF--
+object(EvilStorage)#%d (1) {
+ ["storage":"SplObjectStorage":private]=>
+ array(0) {
+ }
+}
+object(SplObjectStorage)#%d (1) {
+ ["storage":"SplObjectStorage":private]=>
+ array(0) {
+ }
+}