From 28fc06be4908c7763e16c9addc4f3bb2bfb4c450 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Tue, 16 Sep 2025 10:51:07 +0200 Subject: [PATCH 1/3] zend_hash: Assert that the `interned` parameter is not a lie (#19843) * zend_hash: Assert that the `interned` parameter is not a lie While investigating php/php-src#19842 I was wondering why non-interned string didn't cause troubles, until I realized it was the value instead of the key. Nevertheless it appears useful to check that the key is actually interned as claimed by the caller to prevent hard-to-find bugs. * zend_hash: Rename `interned` parameter name to `key_guaranteed_interned` --- Zend/zend_hash.h | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Zend/zend_hash.h b/Zend/zend_hash.h index 71206e61550bb..57020bbcad0b2 100644 --- a/Zend/zend_hash.h +++ b/Zend/zend_hash.h @@ -1621,14 +1621,15 @@ static zend_always_inline bool zend_array_is_list(const zend_array *array) } -static zend_always_inline zval *_zend_hash_append_ex(HashTable *ht, zend_string *key, zval *zv, bool interned) +static zend_always_inline zval *_zend_hash_append_ex(HashTable *ht, zend_string *key, zval *zv, bool key_guaranteed_interned) { uint32_t idx = ht->nNumUsed++; uint32_t nIndex; Bucket *p = ht->arData + idx; ZVAL_COPY_VALUE(&p->val, zv); - if (!interned && !ZSTR_IS_INTERNED(key)) { + ZEND_ASSERT(!key_guaranteed_interned || ZSTR_IS_INTERNED(key)); + if (!key_guaranteed_interned && !ZSTR_IS_INTERNED(key)) { HT_FLAGS(ht) &= ~HASH_FLAG_STATIC_KEYS; zend_string_addref(key); zend_string_hash_val(key); @@ -1647,14 +1648,15 @@ static zend_always_inline zval *_zend_hash_append(HashTable *ht, zend_string *ke return _zend_hash_append_ex(ht, key, zv, 0); } -static zend_always_inline zval *_zend_hash_append_ptr_ex(HashTable *ht, zend_string *key, void *ptr, bool interned) +static zend_always_inline zval *_zend_hash_append_ptr_ex(HashTable *ht, zend_string *key, void *ptr, bool key_guaranteed_interned) { uint32_t idx = ht->nNumUsed++; uint32_t nIndex; Bucket *p = ht->arData + idx; ZVAL_PTR(&p->val, ptr); - if (!interned && !ZSTR_IS_INTERNED(key)) { + ZEND_ASSERT(!key_guaranteed_interned || ZSTR_IS_INTERNED(key)); + if (!key_guaranteed_interned && !ZSTR_IS_INTERNED(key)) { HT_FLAGS(ht) &= ~HASH_FLAG_STATIC_KEYS; zend_string_addref(key); zend_string_hash_val(key); From f045716288af47499fe5c87104e0d3db0570072d Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Mon, 15 Sep 2025 22:07:38 +0200 Subject: [PATCH 2/3] Fix incorrect HASH_FLAG_HAS_EMPTY_IND flag on userland array Fixes GH-19839 Closes GH-19851 --- NEWS | 2 ++ Zend/tests/gh19839.phpt | 18 ++++++++++++++++++ Zend/zend_hash.c | 5 ++++- 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/gh19839.phpt diff --git a/NEWS b/NEWS index 028617dfae40f..0891acce266bf 100644 --- a/NEWS +++ b/NEWS @@ -10,6 +10,8 @@ PHP NEWS exception are triggered). (nielsdos) . Fixed bug GH-19653 (Closure named argument unpacking between temporary closures can cause a crash). (nielsdos, Arnaud, Bob) + . Fixed bug GH-19839 (Incorrect HASH_FLAG_HAS_EMPTY_IND flag on userland + array). (ilutov) - Date: . Fixed GH-17159: "P" format for ::createFromFormat swallows string literals. diff --git a/Zend/tests/gh19839.phpt b/Zend/tests/gh19839.phpt new file mode 100644 index 0000000000000..cc589ce0605f1 --- /dev/null +++ b/Zend/tests/gh19839.phpt @@ -0,0 +1,18 @@ +--TEST-- +GH-19839: Incorrect HASH_FLAG_HAS_EMPTY_IND flag on userland array +--FILE-- + +===DONE=== +--EXPECT-- +===DONE=== diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 07d5bed6d7655..fdc05ff8cbfae 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2466,6 +2466,7 @@ ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(HashTable *source) target->nTableSize = HT_MIN_SIZE; HT_SET_DATA_ADDR(target, &uninitialized_bucket); } else if (GC_FLAGS(source) & IS_ARRAY_IMMUTABLE) { + ZEND_ASSERT(!(HT_FLAGS(source) & HASH_FLAG_HAS_EMPTY_IND)); HT_FLAGS(target) = HT_FLAGS(source) & HASH_FLAG_MASK; target->nTableMask = source->nTableMask; target->nNumUsed = source->nNumUsed; @@ -2482,6 +2483,7 @@ ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(HashTable *source) memcpy(HT_GET_DATA_ADDR(target), HT_GET_DATA_ADDR(source), HT_USED_SIZE(source)); } } else if (HT_IS_PACKED(source)) { + ZEND_ASSERT(!(HT_FLAGS(source) & HASH_FLAG_HAS_EMPTY_IND)); HT_FLAGS(target) = HT_FLAGS(source) & HASH_FLAG_MASK; target->nTableMask = HT_MIN_MASK; target->nNumUsed = source->nNumUsed; @@ -2501,7 +2503,8 @@ ZEND_API HashTable* ZEND_FASTCALL zend_array_dup(HashTable *source) zend_array_dup_packed_elements(source, target, 1); } } else { - HT_FLAGS(target) = HT_FLAGS(source) & HASH_FLAG_MASK; + /* Indirects are removed during duplication, remove HASH_FLAG_HAS_EMPTY_IND accordingly. */ + HT_FLAGS(target) = HT_FLAGS(source) & (HASH_FLAG_MASK & ~HASH_FLAG_HAS_EMPTY_IND); target->nTableMask = source->nTableMask; target->nNextFreeElement = source->nNextFreeElement; target->nInternalPointer = From 7a06f6b4eb473a0f9b97f61e8ca56aa8bb70df4e Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Tue, 16 Sep 2025 13:06:52 +0200 Subject: [PATCH 3/3] [fuzzer][skip ci] Wrap php_request_shutdown() in zend_try (GH-19846) php_request_shutdown() may also bail. E.g. GH-19844. --- sapi/fuzzer/fuzzer-sapi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sapi/fuzzer/fuzzer-sapi.c b/sapi/fuzzer/fuzzer-sapi.c index b1909ef2f42fa..80915d0bbc19f 100644 --- a/sapi/fuzzer/fuzzer-sapi.c +++ b/sapi/fuzzer/fuzzer-sapi.c @@ -220,7 +220,9 @@ void fuzzer_request_shutdown(void) zend_gc_collect_cycles(); } zend_end_try(); - php_request_shutdown(NULL); + zend_try { + php_request_shutdown(NULL); + } zend_end_try(); } /* Set up a dummy stack frame so that exceptions may be thrown. */