menu/materialui: fix leak + null-deref + needless strdup in update_savestate_thumbnail_path

The function had three problems stacked in a handful of lines:

  1. Null-deref: mui->savestate_thumbnail_file_path was dereferenced
     (via strdup) BEFORE the 'if (!mui) return;' guard. If data was
     ever NULL, the guard would never be reached.

  2. Leak: the strdup result was assigned to a const char * and never
     freed on any exit path - not when savestate_thumbnail was
     disabled, not when the entry label check failed, not on the
     success path either. Called frequently (on every selection
     change in the savestate list) so the leak was quickly user-
     visible.

  3. Needless strdup: savestate_thumbnail_file_path is a fixed-size
     char[PATH_MAX_LENGTH] embedded in the struct, not a pointer.
     Heap allocation is unnecessary; a stack buffer of the same size
     does the job and sidesteps both the leak and the OOM failure
     mode (where strdup returning NULL silently flipped the
     string_is_equal comparison below).

Fix: declare a local 'old_path[PATH_MAX_LENGTH]' AFTER the NULL
guard, strlcpy the current path into it before the field is
cleared, and compare against it further down. No heap allocation,
no leak, no dereference before NULL check.

Thread-safety: unchanged. Menu driver callbacks run on the main
thread; materialui_handle_t state is not shared across threads.
The only writes touch mui->savestate_thumbnail_file_path and
mui->thumbnails.savestate, both already single-thread owned.
This commit is contained in:
U-DESKTOP-SPFP6AQ\twistedtechre committed 2026-04-23 18:10:52 +02:00
1 parent 91eba66b26
commit 93449d3ecc
1 file changed
+13 -3
+13 -3
View File
@@ -2484,12 +2484,22 @@ static void materialui_update_savestate_thumbnail_path(void *data, unsigned i)
{
settings_t *settings = config_get_ptr();
materialui_handle_t *mui = (materialui_handle_t*)data;
bool savestate_thumbnail = settings->bools.savestate_thumbnail_enable;
const char *current_path = strdup(mui->savestate_thumbnail_file_path);
bool savestate_thumbnail;
char old_path[PATH_MAX_LENGTH];
if (!mui)
return;
savestate_thumbnail = settings->bools.savestate_thumbnail_enable;
/* Snapshot the current path before clearing it, so we can detect
* whether the resolved path actually changed and only then reset
* the thumbnail. A stack buffer is sufficient (the field it
* mirrors is itself a fixed-size char array) and avoids the leak
* + potential NULL-deref of the previous strdup-before-NULL-check
* construction. */
strlcpy(old_path, mui->savestate_thumbnail_file_path, sizeof(old_path));
mui->savestate_thumbnail_file_path[0] = '\0';
if (savestate_thumbnail)
@@ -2525,7 +2535,7 @@ static void materialui_update_savestate_thumbnail_path(void *data, unsigned i)
strlcpy(mui->savestate_thumbnail_file_path, path,
sizeof(mui->savestate_thumbnail_file_path));
if (!string_is_equal(current_path, mui->savestate_thumbnail_file_path))
if (!string_is_equal(old_path, mui->savestate_thumbnail_file_path))
gfx_thumbnail_reset(&mui->thumbnails.savestate);
materialui_update_fullscreen_thumbnail_label(mui);