From c43c72b960583343c5bee5e78a5cc0a80e4cf2c9 Mon Sep 17 00:00:00 2001 From: jvoisin Date: Wed, 19 Aug 2026 13:31:51 +0200 Subject: [PATCH] Bind free list shadow pointers to the slot holding them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shadow of a free list pointer is currently BSWAP(next) ^ shadow_key, which does not depend on where it is stored, meaning that: - zend_mm_free_small() encodes whatever heap->free_slot[bin] happens to be, including NULL when the bin has been drained, meaning the slot ends up holding shadow_key verbatim. - A (next, shadow) pair harvested from one free slot is valid in every other slot of every bin. An attacker who can read one free slot can therefore forge a link anywhere in the heap without needing the key. This commit adds the address of the slot that holds the shadow into the mix: shadow = BSWAP(next) ^ shadow_key ^ (uintptr_t)holder The holder term cancels on decode, so this is one extra xor on a register that is already live, with no branch. Encoding NULL now yields shadow_key ^ holder rather than the key, and a shadow only verifies in the slot it was written for. This was verified under GDB: Freeing into a drained bin used to store shadow_key exactly; it now stores shadow_key ^ holder (xoring the two back gives the slot address). Naïvely replaying a valid (next, shadow) pair from one slot into another and traversing from it is accepted before this change and aborts with "zend_mm_heap corrupted" after. Performance-wise, the impact is in the noise level, which is expected as it more or less adds a single `xor` instruction per `zend_mm_set_next_free_slot()`. This commit is a follow up on 25360ef2495 and c561f7da858. --- Zend/zend_alloc.c | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/Zend/zend_alloc.c b/Zend/zend_alloc.c index fc7bc1f4d9d4..5824bd971368 100644 --- a/Zend/zend_alloc.c +++ b/Zend/zend_alloc.c @@ -1276,35 +1276,38 @@ static zend_always_inline int zend_mm_small_size_to_bin(size_t size) * before dereference by comparing them with a shadow. * * The shadow is a copy of the pointer, stored at the end of the slot. It is - * XOR'ed with a random key, and converted to big-endian so that smaller - * corruptions affect the most significant bytes, which has a high chance of - * resulting in an invalid address instead of pointing to an adjacent slot. + * XOR'ed with a random key and with the address of the slot holding it, and + * converted to big-endian so that smaller corruptions affect the most + * significant bytes, which has a high chance of resulting in an invalid address + * instead of pointing to an adjacent slot. Mixing in the holder address keeps + * the key from being stored verbatim when the encoded pointer is NULL, and + * prevents a valid shadow from being naïvely replayed into another slot. */ #define ZEND_MM_FREE_SLOT_PTR_SHADOW(free_slot, bin_num) \ *((zend_mm_free_slot**)((char*)(free_slot) + bin_data_size[(bin_num)] - sizeof(zend_mm_free_slot*))) -static zend_always_inline zend_mm_free_slot* zend_mm_encode_free_slot(const zend_mm_heap *heap, const zend_mm_free_slot *slot) +static zend_always_inline zend_mm_free_slot* zend_mm_encode_free_slot(const zend_mm_heap *heap, const zend_mm_free_slot *holder, const zend_mm_free_slot *next) { #ifdef WORDS_BIGENDIAN - return (zend_mm_free_slot*)(((uintptr_t)slot) ^ heap->shadow_key); + return (zend_mm_free_slot*)((uintptr_t)next ^ heap->shadow_key ^ (uintptr_t)holder); #else - return (zend_mm_free_slot*)(BSWAPPTR((uintptr_t)slot) ^ heap->shadow_key); + return (zend_mm_free_slot*)(BSWAPPTR((uintptr_t)next) ^ heap->shadow_key ^ (uintptr_t)holder); #endif } -static zend_always_inline zend_mm_free_slot* zend_mm_decode_free_slot_key(uintptr_t shadow_key, zend_mm_free_slot *slot) +static zend_always_inline zend_mm_free_slot* zend_mm_decode_free_slot_key(uintptr_t shadow_key, const zend_mm_free_slot *holder, zend_mm_free_slot *shadow) { #ifdef WORDS_BIGENDIAN - return (zend_mm_free_slot*)((uintptr_t)slot ^ shadow_key); + return (zend_mm_free_slot*)((uintptr_t)shadow ^ shadow_key ^ (uintptr_t)holder); #else - return (zend_mm_free_slot*)(BSWAPPTR((uintptr_t)slot ^ shadow_key)); + return (zend_mm_free_slot*)(BSWAPPTR((uintptr_t)shadow ^ shadow_key ^ (uintptr_t)holder)); #endif } -static zend_always_inline zend_mm_free_slot* zend_mm_decode_free_slot(zend_mm_heap *heap, zend_mm_free_slot *slot) +static zend_always_inline zend_mm_free_slot* zend_mm_decode_free_slot(zend_mm_heap *heap, const zend_mm_free_slot *holder, zend_mm_free_slot *shadow) { - return zend_mm_decode_free_slot_key(heap->shadow_key, slot); + return zend_mm_decode_free_slot_key(heap->shadow_key, holder, shadow); } static zend_always_inline void zend_mm_set_next_free_slot(zend_mm_heap *heap, uint32_t bin_num, zend_mm_free_slot *slot, zend_mm_free_slot *next) @@ -1312,7 +1315,7 @@ static zend_always_inline void zend_mm_set_next_free_slot(zend_mm_heap *heap, ui ZEND_ASSERT(bin_data_size[bin_num] >= ZEND_MM_MIN_USEABLE_BIN_SIZE); slot->next_free_slot = next; - ZEND_MM_FREE_SLOT_PTR_SHADOW(slot, bin_num) = zend_mm_encode_free_slot(heap, next); + ZEND_MM_FREE_SLOT_PTR_SHADOW(slot, bin_num) = zend_mm_encode_free_slot(heap, slot, next); } static zend_always_inline zend_mm_free_slot *zend_mm_get_next_free_slot(zend_mm_heap *heap, uint32_t bin_num, zend_mm_free_slot* slot) @@ -1320,7 +1323,7 @@ static zend_always_inline zend_mm_free_slot *zend_mm_get_next_free_slot(zend_mm_ zend_mm_free_slot *next = slot->next_free_slot; if (EXPECTED(next != NULL)) { zend_mm_free_slot *shadow = ZEND_MM_FREE_SLOT_PTR_SHADOW(slot, bin_num); - if (UNEXPECTED(next != zend_mm_decode_free_slot(heap, shadow))) { + if (UNEXPECTED(next != zend_mm_decode_free_slot(heap, slot, shadow))) { zend_mm_panic("zend_mm_heap corrupted"); } } @@ -2029,7 +2032,7 @@ ZEND_API void zend_mm_refresh_key_child(zend_mm_heap *heap) zend_mm_free_slot *next; while ((next = slot->next_free_slot)) { zend_mm_free_slot *shadow = ZEND_MM_FREE_SLOT_PTR_SHADOW(slot, i); - if (UNEXPECTED(next != zend_mm_decode_free_slot_key(old_key, shadow))) { + if (UNEXPECTED(next != zend_mm_decode_free_slot_key(old_key, slot, shadow))) { zend_mm_panic("zend_mm_heap corrupted"); } zend_mm_set_next_free_slot(heap, i, slot, next);