menu/xmb: fix five heap-safety issues found during an audit pass

Five independent issues found during a focused audit of menu/drivers/
xmb.c.  Two are real corruption vectors driven by uninitialized memory
and a stale-pointer free ordering; two are size_t underflows in the
ticker helpers that don't fire on the current XMB call sites but
violate the contract a future caller could plausibly hit; and one is
an unused heap allocation on every main-menu init.  Grouped together
because they share an audit context and none is large enough to stand
alone.

* xmb_alloc_node: uninitialized gfx_thumbnail_t fields
  (menu/drivers/xmb.c)

  The comment above xmb_alloc_node deliberately uses malloc instead
  of calloc to avoid zeroing the multi-KB thumbnail_path_data char
  arrays on every node alloc on big lists (MAME, FBA).  Per-field
  init only sets node->thumbnail_icon.icon.texture = 0, leaving the
  rest of the gfx_thumbnail_t substruct (status, flags, alpha,
  delay_timer, width, height) and all of thumbnail_path_data
  uninitialized.

  When a node is later freed without its thumbnail ever having been
  loaded -- the common case for the line-edit path at the bottom of
  the file (xmb_list_init -> xmb_alloc_node, allocated and stored
  into list->list[i].userdata) -- xmb_free_node calls
  gfx_thumbnail_reset, which reads `thumbnail->flags &
  GFX_THUMB_FLAG_FADE_ACTIVE` and `thumbnail->texture` before
  initializing them.  Uninit `flags` may match the bit and trigger
  gfx_animation_kill_by_tag against arbitrary state; uninit
  `texture != 0` feeds a garbage handle to
  video_driver_texture_unload.  The latter is the actual heap
  corruption vector -- driver-side texture maps treat the handle as
  an index/key into their own allocations.

  Separately, the lazy thumbnail-path resolution in the render path
  reads `thumbnail_icon->thumbnail_path_data.icon_path[0]` to decide
  whether resolution is needed; on a freshly-allocated node that
  byte is uninitialized.

  Zero the small gfx_thumbnail_t substruct (~32 bytes) and clear
  icon_path[0] only.  This preserves the original optimization that
  kept thumbnail_path_data's PATH_MAX/NAME_MAX char arrays unzeroed,
  since the path-data fields are written via fill_pathname/strlcpy
  before any read of those fields.

* playlist_db_node_map freed after the nodes it borrows from
  (menu/drivers/xmb.c)

  The map's values are non-owning xmb_node_t* pointers into
  xmb->horizontal_list[i].userdata, populated in
  xmb_context_reset_horizontal_list.  In xmb_refresh_horizontal_list,
  xmb_free, and the xmb_init error path the order was: free nodes
  via xmb_free_list_nodes, then RHMAP_FREE the map.  Between those
  two calls the map holds dangling pointers.

  No re-entry path I could trace would actually read the map during
  that window today (the menu is single-threaded and
  xmb_free_list_nodes calls only gfx_thumbnail_reset and free), but
  the invariant isn't enforced and the fix is a one-line reorder.
  Free the map first at all three sites.

* xmb_animation_line_ticker_smooth: size_t underflow in fade-string
  copy length (menu/drivers/xmb.c)

  At the two fade-string memcpy sites:

      if (copy_len >= line_ticker->top_fade_str_len)
         copy_len = line_ticker->top_fade_str_len - 1;

  if top_fade_str_len == 0 the comparison is true, copy_len
  underflows to SIZE_MAX, the memcpy writes effectively unbounded
  bytes, and the trailing NUL store at line_ticker->top_fade_str
  [copy_len] writes at SIZE_MAX offset.  Same pattern for
  bottom_fade_str_len.

  XMB's only caller passes sizeof(stack_array) so the live path is
  safe today, but xmb_animation_line_ticker_smooth takes the public
  gfx_animation_ctx_line_ticker_smooth_t shape.  Add the missing
  `_str_len > 0` guard to each branch.

* xmb_animation_line_ticker: same underflow on line_ticker->len
  (menu/drivers/xmb.c)

  max_copy = line_ticker->len - 1 underflows when len == 0, then
  copy_len > max_copy is false and the memcpy at line ~4566 writes
  the full block size.  The early sanity check guards line_len < 1
  and max_lines < 1 but not len < 1.  Add it for symmetry with the
  smooth variant; same caller-contract argument.

* xmb_menu_init_list: dead heap allocation
  (menu/drivers/xmb.c)

  info.exts = strldup("lpl", sizeof("lpl")) has been allocated on
  every main-menu init since the function was introduced.
  DISPLAYLIST_MAIN_MENU never reads info->exts (only
  directory-scanning displaylists like
  DISPLAYLIST_DATABASE_PLAYLISTS_HORIZONTAL do, where the same line
  legitimately filters .lpl files); the buffer is allocated, never
  read, then freed by menu_displaylist_info_free.  Almost certainly
  copy-pasted from xmb_init_horizontal_list at line ~2927.  Drop it.

Author: LibretroAdmin

menu/drivers/xmb.c

@@ -640,7 +640,20 @@ const char* xmb_theme_ident(void)
 }
 
 /* NOTE: This exists because calloc()ing xmb_node_t is expensive
- * when you can have big lists like MAME and fba playlists */
+ * when you can have big lists like MAME and fba playlists.
+ *
+ * Per-field init only — DO NOT add a wholesale memset of the
+ * thumbnail_path_data substruct here, that's the multi-KB block
+ * we're explicitly avoiding zeroing on the hot path.  Only the
+ * small `gfx_thumbnail_t icon` substruct and the icon_path's
+ * first byte must be zero, because:
+ *   - xmb_free_node -> gfx_thumbnail_reset reads `texture` and
+ *     `flags` before initializing them; uninit `texture != 0`
+ *     would feed garbage to video_driver_texture_unload, and
+ *     uninit `flags & FADE_ACTIVE` would call
+ *     gfx_animation_kill_by_tag on stale state.
+ *   - The lazy thumbnail path resolution in xmb_render reads
+ *     `icon_path[0]` to decide whether resolution is needed. */
 static xmb_node_t *xmb_alloc_node(void)
 {
    xmb_node_t *node = (xmb_node_t*)malloc(sizeof(*node));
@@ -651,7 +664,9 @@ static xmb_node_t *xmb_alloc_node(void)
    node->alpha        = node->label_alpha  = 0;
    node->zoom         = node->x = node->y  = 0;
    node->icon         = node->content_icon = 0;
-   node->thumbnail_icon.icon.texture       = 0;
+   memset(&node->thumbnail_icon.icon, 0,
+         sizeof(node->thumbnail_icon.icon));
+   node->thumbnail_icon.thumbnail_path_data.icon_path[0] = '\0';
    node->fullpath     = NULL;
    node->console_name = NULL;
 
@@ -3153,9 +3168,14 @@ static void xmb_refresh_horizontal_list(xmb_handle_t *xmb)
 
    xmb_context_destroy_horizontal_list(xmb);
 
+   /* Free the db_node_map BEFORE the nodes it borrows from.
+    * The map's values are non-owning pointers into the
+    * horizontal_list's userdata slots, so once those slots are
+    * free()d the map's stored pointers are dangling.  Reverse
+    * the order to keep the dangling-pointer window closed. */
+   RHMAP_FREE(xmb->playlist_db_node_map);
    xmb_free_list_nodes(&xmb->horizontal_list, false);
    file_list_deinitialize(&xmb->horizontal_list);
-   RHMAP_FREE(xmb->playlist_db_node_map);
 
    menu_st->flags |=  MENU_ST_FLAG_PREVENT_POPULATE;
 
@@ -4445,7 +4465,8 @@ static bool xmb_animation_line_ticker(gfx_animation_t *p_anim, gfx_animation_ctx
       return false;
    if (    (!line_ticker->str || !*line_ticker->str)
        || (line_ticker->line_len  < 1)
-       || (line_ticker->max_lines < 1))
+       || (line_ticker->max_lines < 1)
+       || (line_ticker->len       < 1))
       goto end;
 
    /* Line wrap input string */
@@ -5023,7 +5044,8 @@ static bool xmb_animation_line_ticker_smooth(gfx_animation_t *p_anim, gfx_animat
          bottom_fade_line_index %= num_lines;
       }
 
-      if (top_fade_line_index < num_lines)
+      if (top_fade_line_index < num_lines
+            && line_ticker->top_fade_str_len > 0)
       {
          size_t copy_len = line_lengths[top_fade_line_index];
          if (copy_len >= line_ticker->top_fade_str_len)
@@ -5033,7 +5055,8 @@ static bool xmb_animation_line_ticker_smooth(gfx_animation_t *p_anim, gfx_animat
          line_ticker->top_fade_str[copy_len] = '\0';
       }
 
-      if (bottom_fade_line_index < num_lines)
+      if (bottom_fade_line_index < num_lines
+            && line_ticker->bottom_fade_str_len > 0)
       {
          size_t copy_len = line_lengths[bottom_fade_line_index];
          if (copy_len >= line_ticker->bottom_fade_str_len)
@@ -9534,9 +9557,12 @@ static void *xmb_init(void **userdata, bool video_is_threaded)
 error:
    free(menu);
 
+   /* See comment in xmb_refresh_horizontal_list: the map's
+    * values are non-owning pointers into the horizontal_list,
+    * so it must be freed before the nodes are. */
+   RHMAP_FREE(xmb->playlist_db_node_map);
    xmb_free_list_nodes(&xmb->horizontal_list, false);
    file_list_deinitialize(&xmb->horizontal_list);
-   RHMAP_FREE(xmb->playlist_db_node_map);
    return NULL;
 }
 
@@ -9551,9 +9577,11 @@ static void xmb_free(void *data)
       xmb_icon_load_gen++;
       xmb_ctx_icon_load_gen++;
 
+      /* See comment in xmb_refresh_horizontal_list: free the
+       * db_node_map before the nodes its values point into. */
+      RHMAP_FREE(xmb->playlist_db_node_map);
       xmb_free_list_nodes(&xmb->horizontal_list, false);
       file_list_deinitialize(&xmb->horizontal_list);
-      RHMAP_FREE(xmb->playlist_db_node_map);
 
       video_coord_array_free(&xmb->raster_block.carr);
       video_coord_array_free(&xmb->raster_block2.carr);
@@ -10038,7 +10066,6 @@ static bool xmb_menu_init_list(void *data)
    menu_displaylist_info_init(&info);
 
    info.label                 = strdup(msg_hash_to_str(MENU_ENUM_LABEL_MAIN_MENU));
-   info.exts                  = strldup("lpl", sizeof("lpl"));
    info.type_default          = FILE_TYPE_PLAIN;
    info.enum_idx              = MENU_ENUM_LABEL_MAIN_MENU;