Immediate-Mode-UI / Immediate-Mode-UI/Nuklear

Memory safety (use after free) issue in `nk_buffer_realloc`

Open
#768 5 comments 3 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.