[GIT-PULLS] [php-src] PR #23367: Protect the cached chunk list against corruption

[email protected] (jvoisin)
Newsgroups php.git-pulls
Message-ID <[email protected]>
Pull Request: https://github.com/php/php-src/pull/23367
Author: jvoisin

When a chunk becomes empty it is not systematically unmapped: zend_mm_delete_chunk() often keeps it in heap->cached_chunks so a later allocation can reuse it without going back to mmap(). This single-linked list works via chunk->next, stored in free chunks headers, still mapped and writable.

This is the same shape as free list poisoning
(25360ef24951f1c6b83f8bf85fbdcaff4a1a40e1), and the free lists are the only thing currently protected. An overflow reaching a cached chunk header lets an attacker pick the value that zend_mm_alloc_pages() will pop:

    chunk = heap->cached_chunks;
    heap->cached_chunks = chunk->next;   /* fully attacker controlled */

The popped pointer is then handed to zend_mm_chunk_init(), which writes through it and links it into the live chunk list, so a single controlled qword in a cached header turns into an arbitrary write. Given that corrupting one allocator list pointer is basically the technique to exploit CVE-2024-2961 in PHP (https://blog.lexfo.fr/iconv-cve-2024-2961-p1.html and https://blog.lexfo.fr/iconv-cve-2024-2961-p2.html), leaving a second unprotected one next to it is not great.

Give the cached list the same treatment as the small bins: xor the links with heap->shadow_key, and check that the decoded value is chunk aligned before dereferencing it. NULL terminates the list and is chunk aligned, so it needs no special case. All of this is on the chunk allocation and deletion paths, which are cold, so the cost does not matter.

Something that bit me during the development is rekeying: zend_mm_shutdown() calls zend_mm_refresh_key() at the end of every request, but cached chunks deliberately outlive the request, so their links have to be re-encoded with the new key. Same thing in zend_mm_refresh_key_child() for the post-fork re-key. What made this a pity to find out was that the test suite does not cover the re-keying, because the CLI serves a single request per process. It only shows up over the built-in server, where omitting the re-encode aborts on the second request.

Testing was done with GDB: force a chunk into the cache, overwrite its link with 0x4141414141414141, then force a pop. Before, the corrupted pointer was accepted silently and became heap->cached_chunks. After, it aborts with "zend_mm_heap corrupted".
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.