diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 6ae972d92e49c..3e59990742cbe 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -17,7 +17,7 @@ jobs: if: github.repository == 'php/php-src' steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Install dependencies run: pip install -r docs/requirements.txt - name: Check formatting diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index e856e8c1fc9cd..bbe55e02bbe79 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -54,7 +54,7 @@ jobs: runs-on: [self-hosted, gentoo, ppc64] steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: System info @@ -95,7 +95,7 @@ jobs: image: 'alpine:3.20.1' steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: apk @@ -206,7 +206,7 @@ jobs: runs-on: ubuntu-${{ matrix.asan && inputs.asan_ubuntu_version || inputs.ubuntu_version }} steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: Create MSSQL container @@ -306,7 +306,7 @@ jobs: FIREBIRD_PASSWORD: test steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: apt @@ -374,7 +374,7 @@ jobs: runs-on: macos-${{ matrix.os }} steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: brew @@ -449,7 +449,7 @@ jobs: runs-on: ubuntu-22.04 steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: Create MSSQL container @@ -502,7 +502,7 @@ jobs: USE_TRACKED_ALLOC: 1 steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: apt @@ -705,7 +705,7 @@ jobs: runs-on: ubuntu-${{ inputs.ubuntu_version }} steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: Create MSSQL container @@ -768,7 +768,7 @@ jobs: runs-on: ubuntu-${{ inputs.ubuntu_version }} steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: apt @@ -859,7 +859,7 @@ jobs: runs-on: ubuntu-${{ inputs.ubuntu_version }} steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: apt @@ -909,38 +909,38 @@ jobs: CXX: ccache g++ steps: - name: git checkout PHP - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: path: php ref: ${{ inputs.branch }} - name: git checkout apcu - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: krakjoe/apcu path: apcu - name: git checkout imagick - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: Imagick/imagick path: imagick - name: git checkout memcached - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: php-memcached-dev/php-memcached path: memcached - name: git checkout redis - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: phpredis/phpredis path: redis - name: git checkout xdebug if: false - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: xdebug/xdebug path: xdebug - name: git checkout yaml - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: php/pecl-file_formats-yaml path: yaml @@ -1050,7 +1050,7 @@ jobs: - name: git config run: git config --global core.autocrlf false && git config --global core.eol lf - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: Setup @@ -1071,7 +1071,7 @@ jobs: timeout-minutes: 50 steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: ref: ${{ inputs.branch }} - name: FreeBSD diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index 21e093d0cab3e..b0942b4d700ed 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -82,7 +82,7 @@ jobs: timeout-minutes: 50 steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: apt uses: ./.github/actions/apt-x64 - name: System info @@ -166,7 +166,7 @@ jobs: FIREBIRD_PASSWORD: test steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: apt uses: ./.github/actions/apt-x32 - name: ccache @@ -203,7 +203,7 @@ jobs: timeout-minutes: 50 steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: brew uses: ./.github/actions/brew - name: ccache @@ -250,7 +250,7 @@ jobs: - name: git config run: git config --global core.autocrlf false && git config --global core.eol lf - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Setup uses: ./.github/actions/setup-windows - name: Build @@ -264,7 +264,7 @@ jobs: timeout-minutes: 50 steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: fetch-depth: 0 # ASLR can cause a lot of noise due to missed sse opportunities for memcpy @@ -326,7 +326,7 @@ jobs: mysql -uroot -proot -e "CREATE USER 'wordpress'@'localhost' IDENTIFIED BY 'wordpress'; FLUSH PRIVILEGES;" mysql -uroot -proot -e "GRANT ALL PRIVILEGES ON *.* TO 'wordpress'@'localhost' WITH GRANT OPTION;" - name: git checkout benchmarking-data - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: php/benchmarking-data ssh-key: ${{ secrets.BENCHMARKING_DATA_DEPLOY_KEY }} @@ -369,6 +369,6 @@ jobs: timeout-minutes: 50 steps: - name: git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: FreeBSD uses: ./.github/actions/freebsd diff --git a/.github/workflows/real-time-benchmark.yml b/.github/workflows/real-time-benchmark.yml index cf5ad607eafec..65819cd518ea1 100644 --- a/.github/workflows/real-time-benchmark.yml +++ b/.github/workflows/real-time-benchmark.yml @@ -103,21 +103,21 @@ jobs: sudo apt-get update -y sudo apt-get install -y terraform=1.5.7-* - name: Checkout benchmark suite - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: 'kocsismate/php-version-benchmarks' ref: 'main' fetch-depth: 1 path: 'php-version-benchmarks' - name: Checkout php-src (benchmarked version) - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: '${{ env.REPOSITORY }}' ref: '${{ env.COMMIT }}' fetch-depth: 100 path: 'php-version-benchmarks/tmp/php_${{ env.ID }}' - name: Checkout php-src (baseline version) - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: '${{ env.REPOSITORY }}' ref: '${{ env.BASELINE_COMMIT }}' @@ -131,7 +131,7 @@ jobs: rm -rf ./php-version-benchmarks/docs/results - name: Checkout benchmark data if: github.event_name != 'workflow_dispatch' - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: repository: php/real-time-benchmark-data ssh-key: ${{ secrets.PHP_VERSION_BENCHMARK_RESULTS_DEPLOY_KEY }} diff --git a/.github/workflows/root.yml b/.github/workflows/root.yml index 96943a8cfb2aa..16418b1aa1d3a 100644 --- a/.github/workflows/root.yml +++ b/.github/workflows/root.yml @@ -13,7 +13,7 @@ jobs: outputs: branches: ${{ steps.set-matrix.outputs.branches }} steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 with: # Set fetch-depth to 0 to clone the full repository # including all branches. This is required to find diff --git a/NEWS b/NEWS index 1756b8a1ba032..75d6d1f09f7ff 100644 --- a/NEWS +++ b/NEWS @@ -9,6 +9,9 @@ PHP NEWS . Implement #81724 (openssl_cms_encrypt only allows specific ciphers). (Jakub Zelenka) +- Standard: + . Fixed bug GH-16649 (UAF during array_splice). (alexandre-daubois) + 14 Aug 2025, PHP 8.5.0beta1 - Core: diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index e8a5a4ca7831b..b7396941e5023 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -79,6 +79,7 @@ PHP 8.5 INTERNALS UPGRADE NOTES delayed. Before, errors would be recorded but not delayed. . zend_mm_refresh_key_child() must be called on any zend_mm_heap inherited from the parent process after a fork(). + . HASH_KEY_IS_* constants have been moved in the zend_hash_key_type enum. - standard . ext/standard/php_smart_string.h and ext/standard/php_smart_string_public.h diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 8d3f4d1b68816..f48c298f167e7 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2842,7 +2842,7 @@ ZEND_API zend_result ZEND_FASTCALL zend_hash_move_backwards_ex(const HashTable * /* This function should be made binary safe */ -ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zend_string **str_index, zend_ulong *num_index, const HashPosition *pos) +ZEND_API zend_hash_key_type ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zend_string **str_index, zend_ulong *num_index, const HashPosition *pos) { uint32_t idx; Bucket *p; @@ -2889,7 +2889,7 @@ ZEND_API void ZEND_FASTCALL zend_hash_get_current_key_zval_ex(const HashTable *h } } -ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_type_ex(const HashTable *ht, const HashPosition *pos) +ZEND_API zend_hash_key_type ZEND_FASTCALL zend_hash_get_current_key_type_ex(const HashTable *ht, const HashPosition *pos) { uint32_t idx; Bucket *p; diff --git a/Zend/zend_hash.h b/Zend/zend_hash.h index b2aaecce0d27c..71206e61550bb 100644 --- a/Zend/zend_hash.h +++ b/Zend/zend_hash.h @@ -26,9 +26,11 @@ #include "zend_string.h" #include "zend_sort.h" -#define HASH_KEY_IS_STRING 1 -#define HASH_KEY_IS_LONG 2 -#define HASH_KEY_NON_EXISTENT 3 +typedef enum { + HASH_KEY_IS_STRING = 1, + HASH_KEY_IS_LONG, + HASH_KEY_NON_EXISTENT +} zend_hash_key_type; #define HASH_UPDATE (1<<0) /* Create new entry, or update the existing one. */ #define HASH_ADD (1<<1) /* Create new entry, or fail if it exists. */ @@ -251,9 +253,9 @@ ZEND_API HashPosition ZEND_FASTCALL zend_hash_get_current_pos(const HashTable *h ZEND_API zend_result ZEND_FASTCALL zend_hash_move_forward_ex(const HashTable *ht, HashPosition *pos); ZEND_API zend_result ZEND_FASTCALL zend_hash_move_backwards_ex(const HashTable *ht, HashPosition *pos); -ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zend_string **str_index, zend_ulong *num_index, const HashPosition *pos); +ZEND_API zend_hash_key_type ZEND_FASTCALL zend_hash_get_current_key_ex(const HashTable *ht, zend_string **str_index, zend_ulong *num_index, const HashPosition *pos); ZEND_API void ZEND_FASTCALL zend_hash_get_current_key_zval_ex(const HashTable *ht, zval *key, const HashPosition *pos); -ZEND_API int ZEND_FASTCALL zend_hash_get_current_key_type_ex(const HashTable *ht, const HashPosition *pos); +ZEND_API zend_hash_key_type ZEND_FASTCALL zend_hash_get_current_key_type_ex(const HashTable *ht, const HashPosition *pos); ZEND_API zval* ZEND_FASTCALL zend_hash_get_current_data_ex(const HashTable *ht, const HashPosition *pos); ZEND_API void ZEND_FASTCALL zend_hash_internal_pointer_reset_ex(const HashTable *ht, HashPosition *pos); ZEND_API void ZEND_FASTCALL zend_hash_internal_pointer_end_ex(const HashTable *ht, HashPosition *pos); @@ -270,13 +272,13 @@ static zend_always_inline zend_result zend_hash_move_forward(HashTable *ht) { static zend_always_inline zend_result zend_hash_move_backwards(HashTable *ht) { return zend_hash_move_backwards_ex(ht, &ht->nInternalPointer); } -static zend_always_inline int zend_hash_get_current_key(const HashTable *ht, zend_string **str_index, zend_ulong *num_index) { +static zend_always_inline zend_hash_key_type zend_hash_get_current_key(const HashTable *ht, zend_string **str_index, zend_ulong *num_index) { return zend_hash_get_current_key_ex(ht, str_index, num_index, &ht->nInternalPointer); } static zend_always_inline void zend_hash_get_current_key_zval(const HashTable *ht, zval *key) { zend_hash_get_current_key_zval_ex(ht, key, &ht->nInternalPointer); } -static zend_always_inline int zend_hash_get_current_key_type(const HashTable *ht) { +static zend_always_inline zend_hash_key_type zend_hash_get_current_key_type(const HashTable *ht) { return zend_hash_get_current_key_type_ex(ht, &ht->nInternalPointer); } static zend_always_inline zval* zend_hash_get_current_data(const HashTable *ht) { diff --git a/Zend/zend_weakrefs.c b/Zend/zend_weakrefs.c index cf363cd12595c..fb564a99e2528 100644 --- a/Zend/zend_weakrefs.c +++ b/Zend/zend_weakrefs.c @@ -648,7 +648,7 @@ static void zend_weakmap_iterator_get_current_key(zend_object_iterator *obj_iter zend_string *string_key; zend_ulong num_key; - int key_type = zend_hash_get_current_key_ex(&wm->ht, &string_key, &num_key, pos); + zend_hash_key_type key_type = zend_hash_get_current_key_ex(&wm->ht, &string_key, &num_key, pos); if (key_type == HASH_KEY_NON_EXISTENT) { ZVAL_NULL(key); return; diff --git a/ext/com_dotnet/com_variant.c b/ext/com_dotnet/com_variant.c index 23cbd078fb472..7e63d66b3d3a1 100644 --- a/ext/com_dotnet/com_variant.c +++ b/ext/com_dotnet/com_variant.c @@ -32,7 +32,7 @@ static void safe_array_from_zval(VARIANT *v, zval *z, int codepage) SAFEARRAY *sa = NULL; SAFEARRAYBOUND bound; HashPosition pos; - int keytype; + zend_hash_key_type keytype; zend_string *strindex; zend_ulong intindex = 0; VARIANT *va; diff --git a/ext/com_dotnet/com_wrapper.c b/ext/com_dotnet/com_wrapper.c index 6e885fa802e9f..3d05e17affae7 100644 --- a/ext/com_dotnet/com_wrapper.c +++ b/ext/com_dotnet/com_wrapper.c @@ -423,7 +423,7 @@ static void generate_dispids(php_dispatchex *disp) HashPosition pos; zend_string *name = NULL; zval *tmp, tmp2; - int keytype; + zend_hash_key_type keytype; zend_long pid; if (disp->dispid_to_name == NULL) { diff --git a/ext/opcache/tests/jit/assign_obj_006.phpt b/ext/opcache/tests/jit/assign_obj_006.phpt new file mode 100644 index 0000000000000..a48299f040152 --- /dev/null +++ b/ext/opcache/tests/jit/assign_obj_006.phpt @@ -0,0 +1,37 @@ +--TEST-- +JIT ASSIGN_OBJ: violation of dominance +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.file_update_protection=0 +opcache.jit_buffer_size=1M +--FILE-- +value = $value['value']; + } + return $item; + }, + null, + CacheItem::class); + return $test($value); +} + +$values = [['value'=>'str'], ['value'=>'str'], ['value'=>42]]; +$n = count($values); + +for ($i = 0; $i < $n; $i++) { + test($values[$i]); +} +?> +OK +--EXPECT-- +OK diff --git a/ext/standard/array.c b/ext/standard/array.c index 227d80653846d..981bda9a22254 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -3318,6 +3318,9 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H zval *entry; /* Hash entry */ uint32_t iter_pos = zend_hash_iterators_lower_pos(in_hash, 0); + GC_ADDREF(in_hash); + HT_ALLOW_COW_VIOLATION(in_hash); /* Will be reset when setting the flags for in_hash */ + /* Get number of entries in the input hash */ num_in = zend_hash_num_elements(in_hash); @@ -3485,6 +3488,15 @@ static void php_splice(HashTable *in_hash, zend_long offset, zend_long length, H HT_SET_ITERATORS_COUNT(&out_hash, HT_ITERATORS_COUNT(in_hash)); HT_SET_ITERATORS_COUNT(in_hash, 0); in_hash->pDestructor = NULL; + + if (UNEXPECTED(GC_DELREF(in_hash) == 0)) { + /* Array was completely deallocated during the operation */ + zend_array_destroy(in_hash); + zend_hash_destroy(&out_hash); + zend_throw_error(NULL, "Array was modified during array_splice operation"); + return; + } + zend_hash_destroy(in_hash); HT_FLAGS(in_hash) = HT_FLAGS(&out_hash); diff --git a/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt b/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt new file mode 100644 index 0000000000000..59c0b316f54d5 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_normal_destructor.phpt @@ -0,0 +1,20 @@ +--TEST-- +GH-16649: array_splice with normal destructor should work fine +--FILE-- + +--EXPECT-- +Destructor called +array(1) { + [0]=> + string(1) "1" +} diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt new file mode 100644 index 0000000000000..f5e538122f415 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_add_elements.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-16649: array_splice UAF when destructor adds elements to array +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt new file mode 100644 index 0000000000000..1874ddad8934d --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_array_deallocated.phpt @@ -0,0 +1,22 @@ +--TEST-- +GH-16649: array_splice UAF when array is released entirely +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt new file mode 100644 index 0000000000000..8ad4133300ec5 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_complex_modification.phpt @@ -0,0 +1,25 @@ +--TEST-- +GH-16649: array_splice UAF with complex array modification +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt new file mode 100644 index 0000000000000..9e2c6cf8fc715 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_multiple_destructors.phpt @@ -0,0 +1,33 @@ +--TEST-- +GH-16649: array_splice UAF with multiple destructors +--FILE-- +id = $id; + } + + function __destruct() { + global $arr; + echo "Destructor {$this->id} called\n"; + if ($this->id == 2) { + $arr = null; + } + } +} + +$arr = ["start", new MultiDestructor(1), new MultiDestructor(2), "end"]; + +try { + array_splice($arr, 1, 2); + echo "ERROR: Should have thrown exception\n"; +} catch (Error $e) { + echo "Exception caught: " . $e->getMessage() . "\n"; +} +?> +--EXPECT-- +Destructor 1 called +Destructor 2 called +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt new file mode 100644 index 0000000000000..4a82d5893157d --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_original_case.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-16649: array_splice UAF with destructor modifying array (original case) +--FILE-- + "1", "3" => new C, "2" => "2"]; + +try { + array_splice($arr, 1, 2); + echo "ERROR: Should have thrown exception\n"; +} catch (Error $e) { + echo "Exception caught: " . $e->getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt b/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt new file mode 100644 index 0000000000000..bd8f511b6253e --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_uaf_packed_to_hash.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-16649: array_splice UAF when array is converted from packed to hash +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation diff --git a/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt b/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt new file mode 100644 index 0000000000000..8754a913a24e4 --- /dev/null +++ b/ext/standard/tests/array/gh16649/array_splice_with_replacement.phpt @@ -0,0 +1,23 @@ +--TEST-- +GH-16649: array_splice with replacement array when destructor modifies array +--FILE-- +getMessage() . "\n"; +} +?> +--EXPECT-- +Exception caught: Array was modified during array_splice operation