Immediate-Mode-UI / Immediate-Mode-UI/Nuklear
Memory safety (use after free) issue in `nk_buffer_realloc`
- Dominant language
- C
- Stars
- 11.4k
- Forks
- 686
- Avg merge
- 4d 1h
- Merged PRs (30d)
- 3
Description
## Actual Behavior
The `nk_allocator` interface requires an allocation function which takes a pointer as argument. This suggests that we should use `realloc()` if said pointer is not `NULL` (but the default implementation actually ignores this argument, and simply use `malloc()`).
In the function `nk_buffer_realloc()`, there is a call to the allocation function which passes the buffer memory as argument. But then, if the result of this function is different from the old buffer memory pointer, the old pointer is freed:
https://github.com/Immediate-Mode-UI/Nuklear/blob/master/nuklear.h#L8592-L8595
This is introduce a memory safety issue (use after free). The `realloc()` function might return **the same memory block** at a **different address**:
https://manpages.org/realloc/3
> The `realloc()` function returns a pointer to the newly allocated memory, which is suitably aligned for any built-in type and **may be different from `ptr`**, or `NULL` if the request fails.
This means that if `realloc()` moves the pointer (and it often does), the `nk_buffer_realloc()` will free what it expects to be another allocation, and then the rest of the lib will try to use an allocation that was actually freed.
## Desired Behavior
**Option 1:** You should only check for `NULL` and trust the behavior of `nk_buffer_realloc()` (which would rely on `realloc()` instead of `malloc()`, no more free+memcpy.
**Option 2:** Remove the `ptr` argument of the `nk_plugin_alloc` callback, so that it is 100% clear that `realloc()` is not expected here.
Contributor guide
Research direction
Read nuklear.h around lines 8592-8595 and inspect nk_buffer_realloc together with the nk_plugin_alloc callback contract. Trace how the supplied buffer pointer is handled by the allocator and by callers, then verify that the chosen contract no longer permits the reported use-after-free behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100