-
Notifications
You must be signed in to change notification settings - Fork 670
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 fromptr, orNULLif 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.