View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0010296 | libmicrohttpd | other | public | 2025-08-27 18:51 | 2025-08-27 19:20 |
Reporter | arthurscchan | Assigned To | |||
Priority | normal | Severity | minor | Reproducibility | always |
Status | new | Resolution | open | ||
Summary | 0010296: Possible memory-leaking bug discovered in `mempool_funcs.c` | ||||
Description | The problematic code is in the `mhd_pool_create` function found in `src/mhd2/mempool_funcs.c` (https://git.gnunet.org/libmicrohttpd2.git/tree/src/mhd2/mempool_funcs.c#n293). The snippet below uses several macro-definition checks, so the compiler may produce different variants depending on which macros are defined. ``` #ifdef mhd_USE_LARGE_ALLOCS pool->is_large_alloc = false; if ( (max <= 32 * 1024) || (max < MHD_sys_page_size_ * 4 / 3) ) { pool->memory = (uint8_t *) mhd_MAP_FAILED; } else { /* Round up allocation to page granularity. */ alloc_size = max + MHD_sys_page_size_ - 1; alloc_size -= alloc_size % MHD_sys_page_size_; pool->is_large_alloc = true; # if defined(mhd_MAP_ANONYMOUS) pool->memory = (uint8_t *) mmap (NULL, alloc_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); # else /* ! mhd_MAP_ANONYMOUS */ pool->memory = (uint8_t *) VirtualAlloc (NULL, alloc_size, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE); # endif /* ! mhd_MAP_ANONYMOUS */ } #else /* !mhd_USE_LARGE_ALLOCS */ if (mhd_MAP_FAILED != pool->memory) pool->is_large_alloc = true; else #endif /* !mhd_USE_LARGE_ALLOCS*/ if (! 0) { alloc_size = mhd_ROUND_TO_ALIGN (max); if (MHD_MEMPOOL_ZEROING_NEVER == zeroing) pool->memory = (uint8_t *) malloc (alloc_size); else pool->memory = (uint8_t *) mhd_calloc (1, alloc_size); if (((uint8_t *) NULL) == pool->memory) { free (pool); return NULL; } } ``` When **both** `mhd_USE_LARGE_ALLOCS` and `mhd_MAP_ANONYMOUS` are defined (as in the OSS-Fuzz Linux enviroment), the effective logic reduces to: ``` pool->is_large_alloc = false; if ( (max <= 32 * 1024) || (max < MHD_sys_page_size_ * 4 / 3) ) { pool->memory = (uint8_t *) mhd_MAP_FAILED; } else { /* Round up allocation to page granularity. */ alloc_size = max + MHD_sys_page_size_ - 1; alloc_size -= alloc_size % MHD_sys_page_size_; pool->is_large_alloc = true; pool->memory = (uint8_t *) mmap (NULL, alloc_size, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); } if (! 0) { alloc_size = mhd_ROUND_TO_ALIGN (max); if (MHD_MEMPOOL_ZEROING_NEVER == zeroing) pool->memory = (uint8_t *) malloc (alloc_size); else pool->memory = (uint8_t *) mhd_calloc (1, alloc_size); if (((uint8_t *) NULL) == pool->memory) { free (pool); return NULL; } } ``` If `max` is ≤ `32 * 1024` **or** `< (MHD_sys_page_size_ * 4 / 3)`, the code follows the small-allocation path and later allocates via `malloc`/`mhd_calloc`, which is fine. However, when `max` exceeds `32 * 1024` (and meets the page-sized threshold), a bug appears: `pool->is_large_alloc` is set to `true` and `pool->memory` is allocated with `mmap`. Immediately afterwards, the unconditional `if (!0)` block runs, and `pool->memory` is reassigned to a new `malloc`/`mhd_calloc` allocation **without** freeing the original `mmap` region. This creates the **first memory leak**. Although the `mmap` region will be automatically `munmap` when process terminate, that region still reamin unusable until the process really terminate and create a memory leakage problem. More seriously, when the pool is later destroyed by `mhd_pool_destroy` (https://git.gnunet.org/libmicrohttpd2.git/tree/src/mhd2/mempool_funcs.c#n362), a **second** issue is triggered: ``` MHD_INTERNAL void mhd_pool_destroy (struct mhd_MemoryPool *restrict pool) { if (NULL == pool) return; mhd_assert (pool->end >= pool->pos); mhd_assert (pool->size >= pool->end - pool->pos); mhd_assert (pool->pos == mhd_ROUND_TO_ALIGN (pool->pos)); mhd_UNPOISON_MEMORY (pool->memory, pool->size); #ifdef mhd_USE_LARGE_ALLOCS if (pool->is_large_alloc) { # if defined(mhd_MAP_ANONYMOUS) munmap (pool->memory, pool->size); # else VirtualFree (pool->memory, 0, MEM_RELEASE); # endif } else #endif /* mhd_USE_LARGE_ALLOCS*/ if (! 0) free (pool->memory); free (pool); } ``` With `mhd_USE_LARGE_ALLOCS` and `mhd_MAP_ANONYMOUS` defined, the effective logic becomes: ``` MHD_INTERNAL void mhd_pool_destroy (struct mhd_MemoryPool *restrict pool) { if (NULL == pool) return; mhd_assert (pool->end >= pool->pos); mhd_assert (pool->size >= pool->end - pool->pos); mhd_assert (pool->pos == mhd_ROUND_TO_ALIGN (pool->pos)); mhd_UNPOISON_MEMORY (pool->memory, pool->size); if (pool->is_large_alloc) { munmap (pool->memory, pool->size); } else if (! 0) free (pool->memory); free (pool); } ``` Because `mhd_pool_create` set `pool->is_large_alloc = true` but then **overwrote** `pool->memory` with a heap pointer, `mhd_pool_destroy` ends up calling `munmap` on memory that was allocated by `malloc` or `mhd_calloc`. This is undefined behvaiour, it may crash, silently no-op, or corrupt memory. In the latter two cases, the heap allocation is never freed, which is how the **second leak** occurs. In short, a large-allocation path leaves an `mmap` region leaked (first leak), and the destroy path may call `munmap` on a heap pointer allocated by `malloc` or `mhd_calloc` while failing to `free` it, causing a seconday leak. This likely occured only when the two macros above are enabled simultanously. | ||||
Steps To Reproduce | Defining both mhd_USE_LARGE_ALLOCS and mhd_MAP_ANONYMOUS and set the max size > 32 * 1024 will trigger the memory leaking. | ||||
Tags | No tags attached. | ||||
|
Found during the ongoing security audit carried out by Ada Logics and facilitated by OSTIF in the libmicrohttpd2 project. |
Date Modified | Username | Field | Change |
---|---|---|---|
2025-08-27 18:51 | arthurscchan | New Issue | |
2025-08-27 19:20 | arthurscchan | Note Added: 0025703 |