From 383b5bb7c579709fbb635fbea992b89307ec47cf Mon Sep 17 00:00:00 2001 From: jvoisin Date: Tue, 18 Aug 2026 23:17:12 +0200 Subject: [PATCH 1/4] Detect immediate double-frees of zend_mm small slots Freeing the same small pointer twice in a row pushed it onto the freelist twice, so the next two allocations of that bin returned the same address. That's a nifty primitive to obtain two live pointers of different types to the same object. The shadow-pointer check does not catch it, as both links are consistent. This commit adds a simple check for when the freed pointer already is the head of the freelist. heap->free_slot[bin_num] is loaded by the very next line, so the check costs a single comparison on an already-hot value. This only catches consecutive double-frees, not a free after other activity on the same bin, but it doesn't cost ~anything performance wise, and catches real bugs like error/cleanup paths freeing the same value twice. A quick look at `git log --grep='double.free'` shows that this is a popular bug pattern. --- Zend/zend_alloc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index 575b54b11a24..02de1a543da9 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -1430,6 +1430,12 @@ static zend_always_inline void zend_mm_free_small(zend_mm_heap *heap, void *ptr, #endif p = (zend_mm_free_slot*)ptr; +#if ZEND_MM_HEAP_PROTECTION + /* Catch the most common double-free pattern for free. */ + if (UNEXPECTED(p == heap->free_slot[bin_num])) { + zend_mm_panic("zend_mm_heap corrupted (double free)"); + } +#endif zend_mm_set_next_free_slot(heap, bin_num, p, heap->free_slot[bin_num]); heap->free_slot[bin_num] = p; } From 9d6fd4fd98aa9e7662f7b8082ce328b20c6be7b6 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Wed, 26 Aug 2026 18:25:39 +0200 Subject: [PATCH 2/4] Fix double free of phpdbg watch element chains PHPDBG_G(watch_recreation) is keyed by element->str, so a watch element and its implicit parent are stored under distinct keys ("$lower[0]" and "$lower[]"). The deduplication in phpdbg_queue_element_for_recreation() only walks down from the entry found under the *same* key, so it never notices that the two belong to the same chain and both get queued. phpdbg_free_watch_element_tree() frees the element together with its entire parent and child chains, so draining watch_recreation freed that chain twice: the first entry frees the whole chain, the second one then walks the already freed links and frees them again. Drop every entry referencing a member of the chain before freeing it. The buckets are nulled out rather than deleted, as all callers are iterating over the hash at that point; they clean it right afterwards. --- sapi/phpdbg/phpdbg_watch.c | 40 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c index 1e67d2c57670..a2e39e15a6ca 100644 --- a/sapi/phpdbg/phpdbg_watch.c +++ b/sapi/phpdbg/phpdbg_watch.c @@ -847,8 +847,36 @@ bool phpdbg_try_re_adding_watch_element(zval *parent, phpdbg_watch_element *elem return true; } +/* watch_recreation is keyed by element->str, so a parent and its child are stored + * under distinct keys and the deduplication in phpdbg_queue_element_for_recreation() + * cannot notice that they belong to the same chain. As phpdbg_free_watch_element_tree() + * frees the whole chain, any other entry referencing a member of it must be dropped + * first, or the chain gets freed twice. The buckets are only nulled out as the hash is + * being iterated over by the callers; they clean it right after. */ +static void phpdbg_forget_queued_watch_element(phpdbg_watch_element *element) { + zval *zv = zend_hash_find(&PHPDBG_G(watch_recreation), element->str); + if (zv && Z_PTR_P(zv) == element) { + Z_PTR_P(zv) = NULL; + } +} + +static void phpdbg_dequeue_watch_element_tree(phpdbg_watch_element *element) { + phpdbg_watch_element *cur; + + for (cur = element->parent; cur; cur = cur->parent) { + phpdbg_forget_queued_watch_element(cur); + } + for (cur = element->child; cur; cur = cur->child) { + phpdbg_forget_queued_watch_element(cur); + } + phpdbg_forget_queued_watch_element(element); +} + void phpdbg_automatic_dequeue_free(phpdbg_watch_element *element) { phpdbg_watch_element *child = element; + + phpdbg_dequeue_watch_element_tree(element); + while (child->child && !(child->flags & PHPDBG_WATCH_RECURSIVE_ROOT)) { child = child->child; } @@ -863,6 +891,10 @@ void phpdbg_dequeue_elements_for_recreation(void) { phpdbg_watch_element *element; ZEND_HASH_MAP_FOREACH_PTR(&PHPDBG_G(watch_recreation), element) { + if (!element) { + /* freed along with an already dequeued element of the same chain */ + continue; + } ZEND_ASSERT(element->flags & (PHPDBG_WATCH_IMPLICIT | PHPDBG_WATCH_RECURSIVE_ROOT | PHPDBG_WATCH_SIMPLE)); if (element->parent || zend_hash_index_find(&PHPDBG_G(watch_free), (zend_ulong)(uintptr_t) element->parent_container)) { zval _zv, *zv = &_zv; @@ -1641,7 +1673,9 @@ void phpdbg_destroy_watchpoints(void) { /* unconditionally free all remaining elements to avoid memory leaks */ ZEND_HASH_MAP_FOREACH_PTR(&PHPDBG_G(watch_recreation), element) { - phpdbg_automatic_dequeue_free(element); + if (element) { + phpdbg_automatic_dequeue_free(element); + } } ZEND_HASH_FOREACH_END(); /* upon fatal errors etc. (i.e. CG(unclean_shutdown) == 1), some watchpoints may still be active. Ensure memory is not watched anymore for next run. Do not care about memory freeing here, shutdown is unclean and near anyway. */ @@ -1669,7 +1703,9 @@ void phpdbg_release_watch_elements(void) { uint32_t guard; ZEND_HASH_MAP_FOREACH_PTR(&PHPDBG_G(watch_recreation), element) { - phpdbg_automatic_dequeue_free(element); + if (element) { + phpdbg_automatic_dequeue_free(element); + } } ZEND_HASH_FOREACH_END(); zend_hash_clean(&PHPDBG_G(watch_recreation)); From afc3ee574929606ca74b8bc872484358c644656b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Thu, 27 Aug 2026 13:51:54 +0200 Subject: [PATCH 3/4] Expose the "binary layout strategy" option for the real-time benchmark See https://github.com/kocsismate/php-version-benchmarks/commit/351e967b1 for details. "bolt_align" (https://github.com/llvm/llvm-project/pull/210634) is an experimental option, but it should basically eliminate any false positive differences due to binary layout changes. The "bolt" option stands for regular BOLT compilation. --- .github/workflows/real-time-benchmark.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/real-time-benchmark.yml b/.github/workflows/real-time-benchmark.yml index b017a1b67c07..559eba5fd0b0 100644 --- a/.github/workflows/real-time-benchmark.yml +++ b/.github/workflows/real-time-benchmark.yml @@ -16,6 +16,15 @@ on: options: - "0" - "1" + binary_layout_strategy: + description: 'How the binary layout is generated' + required: false + default: "" + type: choice + options: + - "" + - "bolt_align" + - "bolt" collect_extended_perf_stats: description: 'Whether to collect extended perf stats as artifacts' required: true @@ -58,6 +67,7 @@ jobs: BASELINE_COMMIT: "d5f6e56610c729710073350af318c4ea1b292cfe" ID: "master" JIT: "1" + BINARY_LAYOUT_STRATEGY: "" COLLECT_EXTENDED_PERF_STATS: "0" DEBUG_ENVIRONMENT: "0" RUN_MICRO_BENCH: "0" @@ -90,6 +100,7 @@ jobs: echo "ID=benchmarked" >> $GITHUB_ENV echo "JIT=${{ inputs.jit }}" >> $GITHUB_ENV + echo "BINARY_LAYOUT_STRATEGY=${{ inputs.binary_layout_strategy }}" >> $GITHUB_ENV echo "COLLECT_EXTENDED_PERF_STATS=${{ inputs.collect_extended_perf_stats }}" >> $GITHUB_ENV echo "DEBUG_ENVIRONMENT=${{ inputs.debug_environment }}" >> $GITHUB_ENV echo "RUN_MICRO_BENCH=${{ inputs.run_micro_bench }}" >> $GITHUB_ENV @@ -231,6 +242,7 @@ jobs: cp ./php-version-benchmarks/config/infra/aws/x86_64-metal.ini.dist ./php-version-benchmarks/config/infra/aws/x86_64-metal.ini sed -i "s|INFRA_DOCKER_REGISTRY=public.ecr.aws/abcdefgh|INFRA_DOCKER_REGISTRY=${{ secrets.PHP_VERSION_BENCHMARK_DOCKER_REGISTRY }}|g" ./php-version-benchmarks/config/infra/aws/x86_64-metal.ini sed -i "s|INFRA_WORKSPACE=|INFRA_WORKSPACE=$WORKSPACE|g" ./php-version-benchmarks/config/infra/aws/x86_64-metal.ini + sed -i "s/INFRA_BINARY_LAYOUT_STRATEGY=/INFRA_BINARY_LAYOUT_STRATEGY=${{ env.BINARY_LAYOUT_STRATEGY }}/g" ./php-version-benchmarks/config/infra/aws/x86_64-metal.ini sed -i "s/INFRA_COLLECT_EXTENDED_PERF_STATS=0/INFRA_COLLECT_EXTENDED_PERF_STATS=${{ env.COLLECT_EXTENDED_PERF_STATS }}/g" ./php-version-benchmarks/config/infra/aws/x86_64-metal.ini sed -i "s/INFRA_DEBUG_ENVIRONMENT=0/INFRA_DEBUG_ENVIRONMENT=${{ env.DEBUG_ENVIRONMENT }}/g" ./php-version-benchmarks/config/infra/aws/x86_64-metal.ini From 8196275133edb830ec85330d36659d5b257b44db Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Mon, 24 Aug 2026 12:05:47 -0400 Subject: [PATCH 4/4] [http] Fix out-of-bounds read on empty Location header An empty Location header allocates a single byte for the NUL terminator, so reading location[1] in the relative-redirect branch over-reads heap memory and could append a garbage-derived path to the redirect target instead of the correct host root. Use location_len instead of strlen, and skip the relative join when location_len is 0, so the second byte is never read. Closes GH-23467 --- NEWS | 2 ++ ext/standard/http_fopen_wrapper.c | 4 +-- .../http/http_empty_location_redirect.phpt | 36 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) create mode 100644 ext/standard/tests/http/http_empty_location_redirect.phpt diff --git a/NEWS b/NEWS index c9c96543120d..59517a0dd355 100644 --- a/NEWS +++ b/NEWS @@ -59,6 +59,8 @@ PHP NEWS (Weilin Du) - Standard: + . Fixed an out-of-bounds read when following a redirect response with an + empty Location header. (iliaal) . Fixed a memory leak in array_merge_recursive() when the recursive merge of an object converted to an array fails. (David Carlier) diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index 89125ed0765e..9bd12ba527ca 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -1052,7 +1052,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, char *new_path = NULL; - if (strlen(header_info.location) < 8 || + if (header_info.location_len < 8 || (strncasecmp(header_info.location, "http://", sizeof("http://")-1) && strncasecmp(header_info.location, "https://", sizeof("https://")-1) && strncasecmp(header_info.location, "ftp://", sizeof("ftp://")-1) && @@ -1060,7 +1060,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, { char *loc_path = NULL; if (*header_info.location != '/') { - if (*(header_info.location+1) != '\0' && resource->path) { + if (header_info.location_len > 0 && resource->path) { char *s = strrchr(ZSTR_VAL(resource->path), '/'); if (!s) { s = ZSTR_VAL(resource->path); diff --git a/ext/standard/tests/http/http_empty_location_redirect.phpt b/ext/standard/tests/http/http_empty_location_redirect.phpt new file mode 100644 index 000000000000..a7f99bf1e249 --- /dev/null +++ b/ext/standard/tests/http/http_empty_location_redirect.phpt @@ -0,0 +1,36 @@ +--TEST-- +Empty Location header must not over-read when building the redirect target +--FILE-- + ['follow_location' => 1]]); +echo @file_get_contents("http://{{ ADDR }}/a/b", false, $ctx), "\n"; +CODE; + +include sprintf("%s/../../../openssl/tests/ServerClientTestCase.inc", __DIR__); +ServerClientTestCase::getInstance()->run($clientCode, $serverCode); +?> +--EXPECT-- +uri=/