libretro-common/formats: harden rtga + rbmp against hostile image headers

Audit of the TGA (403 lines) and BMP (577 lines) image
decoders.  Both are stb_image-derived and both accept
attacker-controlled bytes from files on disk, over HTTP
(thumbnails, cores, assets bundles), or from user uploads.
Nine fixes this round; one is a directly reachable stack
buffer overflow.

Nothing touches rjpeg.c (4808 lines) or rpng.c (1859 lines);
those are their own rounds.

=== rtga.c ===

rtga: reject bogus palette_bits at the header (CRITICAL)
  A single attacker-controlled byte in the TGA header
  (tga_palette_bits, byte 7) drove a stack buffer overflow.
  The indexed-pixel read loop did:

     for (j = 0; j * 8 < tga_palette_bits; ++j)
        raw_data[j] = tga_palette[pal_idx + j];

  where raw_data is declared as "unsigned char raw_data[4]"
  on the stack.  With tga_palette_bits = 255 the loop writes
  raw_data[0..31] -- up to 28 bytes of stack corruption per
  pixel, driven directly by the header.  Reachable on any
  ingest path that accepts TGA (core-provided asset bundles,
  RetroAchievement badges, user thumbnails, network mirrors).

  Fix: validate tga_palette_bits at the header-check point.
  TGA palettes legitimately hold only 15/16/24/32-bit entries;
  anything else is rejected.  Also reject tga_palette_len < 1
  (empty palette) which pre-patch malloc(0)'d and then read
  tga_palette[0] OOB.

rtga: rtga_skip() clamp
  rtga_skip advanced img_buffer by n without checking against
  img_buffer_end.  Pointer arithmetic past the end of the
  input array is undefined behaviour in C even without a
  deref.  Clamp to the remaining input; subsequent
  rtga_get8 calls check "buffer < buffer_end" and parse as
  EOF.

rtga: size_t output allocation + ceiling
  malloc((size_t)tga_width * tga_height * sizeof(uint32_t))
  was OK on 64-bit but could wrap on 32-bit for the full
  65535 x 65535 header-expressible range.  Also, even when
  the arithmetic doesn't wrap, a legitimate-looking
  65535 x 65535 ARGB decode would request ~17 GiB of memory
  from a single malicious 18-byte header.

  Fix: reject dimensions above 0x4000 x 0x4000 (16384 pixels
  each axis = 1 GiB decoded RGBA, larger than any realistic
  libretro asset).  Do the malloc math in size_t.

rtga: reject size > INT_MAX in rtga_process_image
  rtga_process_image cast "size" (size_t) to int before
  passing it to rtga_load_from_memory.  A file >= 2 GiB
  truncated, propagated a negative len into
  img_buffer_end = buffer + len, and gave pointer arithmetic
  outside the source object.  Reject early.

rtga: size_t indexing in output writes
  output[dst_row * tga_width + cur_col] used signed int for
  the index.  65534 * 65535 overflows int32, UB.  Use size_t.

=== rbmp.c ===

rbmp: reject img_y = 0x80000000 (abs(INT_MIN) UB)
  Pre-patch the code did
      flip_vertically = ((int) s->img_y) > 0;
      s->img_y        = abs((int) s->img_y);
  When img_y == 0x80000000u, the cast to int is INT_MIN and
  abs(INT_MIN) is explicitly UB in C99.  On GCC with 2's-
  complement it returns INT_MIN and the uint32_t reassignment
  leaves img_y == 0x80000000 -- a "negative" height that then
  drives the output allocation to a very large value.

  Fix: detect this specific value before the abs() call and
  bail cleanly.

rbmp: size_t output allocation + ceiling
  malloc(s->img_x * s->img_y * sizeof(uint32_t)) did the
  multiplication in uint32_t (both operands are uint32_t),
  so e.g. 0x10001 x 0x10000 silently wrapped to 0x10000,
  producing a 256 KiB buffer for 16 GiB of claimed image
  data.  The subsequent per-row pixel writes ran off the
  end for gigabytes -- heap corruption driven by a 14-byte
  BMP header.

  Fix: same ceiling as rtga (0x4000 x 0x4000) and size_t
  math for the malloc.

rbmp: rbmp_skip() clamp
  Same as rtga_skip.

rbmp: size_t indexing in output writes
  output + dst_row * s->img_x used signed int * uint32
  (promoted to uint32), which wraps.  Use size_t.

=== Regression tests (two new) ===

libretro-common/samples/formats/tga/rtga_test.c
  1. test_malicious_palette_bits (palette_bits=255) --
     header check must reject.
  2. test_empty_indexed_palette -- TRUE DISCRIMINATOR
     under ASan.  Pre-patch the unpatched decoder fires:
       AddressSanitizer: heap-buffer-overflow
         READ of size 1 at rtga.c:286 in rtga_tga_load
         (1-byte region allocated at rtga.c:234)
     Post-patch the decoder returns IMAGE_PROCESS_ERROR at
     the header check.
  3. test_happy_path_rgb -- 2x2 uncompressed BGR smoke.
  4. test_oversized_size -- size > INT_MAX rejection.

libretro-common/samples/formats/bmp/rbmp_test.c
  1. test_img_y_int_min -- 0x80000000 height, post-patch
     rejection.  Pre-patch would invoke UB via abs(INT_MIN)
     and the downstream allocation decisions are
     unpredictable.
  2. test_dimension_overflow -- 0x10001 x 0x10000, post-
     patch rejection.  The pre-patch 32-bit wrap is not
     reproducible on a 64-bit CI host but the rejection IS
     reproducible everywhere and prevents the 17 GiB
     allocation attempt that would otherwise OOM the test.
  3. test_happy_1x1_24bpp -- happy-path smoke.

Builds clean under -Wall -Werror -std=gnu99.  ASan -O0
clean on patched; the header-check rejections fire before
any memory is touched, so ASan never sees anything.

CI: both new tests added to RUN_TARGETS.  Full samples
workflow dry-run:
    Built: 17  Ran: 17  Failed: 0

VC6 compatibility: both files are compiled on all
platforms.  The fix uses only size_t / uint32_t / int and
the existing stdint shim; <limits.h> (C89) is explicitly
included where needed.  No new dependencies.

Reachability summary:
  * Stack BOF via TGA palette_bits: HIGH severity, single
    header byte, any TGA ingest path.
  * Heap BOF via BMP dimension wrap: MEDIUM severity on
    32-bit, reaches malloc-failure on 64-bit.
  * Other fixes: UB hardening + defense-in-depth.

Author: LibretroAdmin

.github/workflows/Linux-libretro-common-samples.yml

@@ -56,6 +56,8 @@ jobs:
             cdrom_cuesheet_overflow_test
             http_parse_test
             rjson_test
+            rtga_test
+            rbmp_test
           )
 
           # Per-binary run command (overrides ./<binary> if present).

libretro-common/formats/bmp/rbmp.c

@@ -28,6 +28,7 @@
 #include <stddef.h> /* ptrdiff_t on osx */
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h> /* INT_MAX */
 
 #include <retro_inline.h>
 
@@ -66,13 +67,20 @@ static INLINE unsigned char rbmp_get8(rbmp_context *s)
 
 static void rbmp_skip(rbmp_context *s, int n)
 {
+   ptrdiff_t remaining;
    if (n < 0)
    {
       s->img_buffer = s->img_buffer_end;
       return;
    }
-
-   s->img_buffer += n;
+   /* Clamp to remaining input to avoid pointer arithmetic beyond
+    * img_buffer_end (UB per C99).  Subsequent rbmp_get8 calls
+    * check "buffer < buffer_end" and parse as EOF. */
+   remaining = s->img_buffer_end - s->img_buffer;
+   if ((ptrdiff_t)n > remaining)
+      s->img_buffer = s->img_buffer_end;
+   else
+      s->img_buffer += n;
 }
 
 static int rbmp_get16le(rbmp_context *s)
@@ -193,8 +201,16 @@ static uint32_t *rbmp_bmp_load(rbmp_context *s, unsigned *x, unsigned *y,
    if (bpp == 1)
       return 0;
 
+   /* img_y can legitimately be a negative int (top-down BMP).
+    * Pre-patch the code did abs((int)s->img_y) -- if the uint32
+    * happened to be 0x80000000, the cast to int is INT_MIN and
+    * abs(INT_MIN) is undefined behaviour.  Detect that case and
+    * treat as bottom-up zero-height (rejected by the overflow
+    * guard below). */
+   if (s->img_y == 0x80000000u)
+      return 0;
    flip_vertically = ((int) s->img_y) > 0;
-   s->img_y        = abs((int) s->img_y);
+   s->img_y        = (uint32_t)abs((int) s->img_y);
 
    if (hsz == 12)
    {
@@ -298,8 +314,25 @@ static uint32_t *rbmp_bmp_load(rbmp_context *s, unsigned *x, unsigned *y,
    }
    s->img_n = ma ? 4 : 3;
 
-   /* Always output as uint32 (4 bytes per pixel) */
-   output = (uint32_t*)malloc(s->img_x * s->img_y * sizeof(uint32_t));
+   /* Always output as uint32 (4 bytes per pixel).
+    *
+    * Pre-patch this multiplied two uint32_t dimensions at uint32_t
+    * width, so img_x * img_y silently wrapped on 32-bit -- e.g.
+    * 0x10001 * 0x10000 = 0x100010000 wraps to 0x10000, and the
+    * subsequent malloc returned a 256 KiB buffer that the pixel
+    * decode loop wrote off the end of (4 GiB+ of writes).  Do
+    * the multiplication in size_t with an explicit ceiling:
+    * 0x4000 x 0x4000 = 256 M pixels = 1 GiB of decoded RGBA is
+    * far beyond any realistic libretro asset, and rejecting at
+    * that ceiling blocks the overflow case AND avoids giving a
+    * malicious BMP a direct route to a multi-GiB allocation
+    * attempt. */
+   if (s->img_x == 0 || s->img_y == 0)
+      return 0;
+   if (s->img_x > 0x4000u || s->img_y > 0x4000u)
+      return 0;
+   output = (uint32_t*)malloc(
+         (size_t)s->img_x * (size_t)s->img_y * sizeof(uint32_t));
    if (!output)
       return 0;
 
@@ -342,7 +375,11 @@ static uint32_t *rbmp_bmp_load(rbmp_context *s, unsigned *x, unsigned *y,
       for (j = 0; j < (int)s->img_y; ++j)
       {
          int dst_row = flip_vertically ? (int)(s->img_y - 1 - j) : j;
-         uint32_t *dst = output + dst_row * s->img_x;
+         /* Use size_t for the row-stride offset.  dst_row *
+          * s->img_x in signed-int could overflow for a legitimate
+          * 2 GiB BMP (e.g. 46341 x 46341).  Post-patch the
+          * pointer math matches the size_t-based allocation. */
+         uint32_t *dst = output + (size_t)dst_row * (size_t)s->img_x;
          int col = 0;
 
          for (i = 0; i < (int)s->img_x; i += 2)

libretro-common/formats/tga/rtga.c

@@ -28,6 +28,7 @@
 #include <stddef.h> /* ptrdiff_t on osx */
 #include <stdlib.h>
 #include <string.h>
+#include <limits.h> /* INT_MAX, SIZE_MAX via stdint */
 
 #include <retro_inline.h>
 
@@ -60,12 +61,24 @@ static INLINE uint8_t rtga_get8(rtga_context *s)
 
 static void rtga_skip(rtga_context *s, int n)
 {
+   ptrdiff_t remaining;
    if (n < 0)
    {
       s->img_buffer = s->img_buffer_end;
       return;
    }
-   s->img_buffer += n;
+   /* Clamp the advance to the remaining input.  Pre-patch a large
+    * attacker-supplied offset (TGA header byte 1, or palette
+    * start) pushed img_buffer past img_buffer_end, which is
+    * pointer arithmetic outside the allocated object (UB per C99).
+    * All callers of rtga_get8 check "buffer < buffer_end" so the
+    * clamped state parses as EOF and the subsequent header checks
+    * or indexed-palette code fail cleanly. */
+   remaining = s->img_buffer_end - s->img_buffer;
+   if ((ptrdiff_t)n > remaining)
+      s->img_buffer = s->img_buffer_end;
+   else
+      s->img_buffer += n;
 }
 
 static int rtga_get16le(rtga_context *s)
@@ -121,17 +134,48 @@ static uint32_t *rtga_tga_load(rtga_context *s,
       )
       return NULL;
 
-   /*   If paletted, then we will use the number of bits from the palette */
+   /*   If paletted, then we will use the number of bits from the palette.
+    *
+    *   tga_palette_bits is attacker-controlled (TGA byte 7, 0..255).
+    *   Pre-patch the indexed-read loop below did
+    *       for (j = 0; j * 8 < tga_palette_bits; ++j)
+    *           raw_data[j] = tga_palette[pal_idx + j];
+    *   raw_data is a 4-byte stack array, so tga_palette_bits > 32
+    *   wrote past the end of raw_data -- a stack buffer overflow of
+    *   up to 28 bytes, directly driven by the TGA header.  Reject
+    *   bogus palette_bits / empty palettes here and everything
+    *   downstream runs with bounded buffers. */
    if (tga_indexed)
+   {
+      if (    tga_palette_len < 1
+           || (    tga_palette_bits != 15
+                && tga_palette_bits != 16
+                && tga_palette_bits != 24
+                && tga_palette_bits != 32))
+         return NULL;
       tga_comp = tga_palette_bits / 8;
+   }
 
    /*   TGA info */
    *x = tga_width;
    *y = tga_height;
    if (comp)
       *comp = tga_comp;
 
-   output = (uint32_t*)malloc((size_t)tga_width * tga_height * sizeof(uint32_t));
+   /* Bound the output allocation.  TGA dimensions are attacker-
+    * controlled 16-bit values (max 65535), so their product is
+    * up to ~4.29 G pixels.  Pre-patch a 32-bit build could wrap
+    * (size_t)w * h * sizeof(uint32_t) to a small positive size_t
+    * and the per-pixel decode ran off the undersized malloc.
+    * Reject dimensions beyond a sane ceiling so the allocation
+    * never grows anywhere near wrap territory and hostile
+    * headers cannot drive the client into a multi-GiB malloc
+    * attempt.  0x4000 x 0x4000 = 1 GiB of decoded RGBA,
+    * comfortably larger than any real-world libretro asset. */
+   if (tga_width > 0x4000 || tga_height > 0x4000)
+      return NULL;
+   output = (uint32_t*)malloc(
+         (size_t)tga_width * (size_t)tga_height * sizeof(uint32_t));
    if (!output)
       return NULL;
 
@@ -226,19 +270,23 @@ static uint32_t *rtga_tga_load(rtga_context *s,
       int cur_col                = 0;
       int cur_row                = 0;
 
-      /* Load palette if indexed */
+      /* Load palette if indexed.  Header-level checks above have
+       * ensured tga_palette_len >= 1 and tga_palette_bits in
+       * {15,16,24,32}, so n is positive and bounded at
+       * 65535 * 32 / 8 = 262140 bytes -- fits comfortably in int
+       * and in the input length we've already accepted. */
       if (tga_indexed)
       {
          int n;
          rtga_skip(s, tga_palette_start);
-         tga_palette = (unsigned char*)malloc(tga_palette_len * tga_palette_bits / 8);
+         n = tga_palette_len * tga_palette_bits / 8;
+         tga_palette = (unsigned char*)malloc((size_t)n);
          if (!tga_palette)
          {
             free(output);
             return NULL;
          }
-         n = tga_palette_len * tga_palette_bits / 8;
-         if (s->img_buffer + n <= s->img_buffer_end)
+         if (s->img_buffer_end - s->img_buffer >= (ptrdiff_t)n)
          {
             memcpy(tga_palette, s->img_buffer, n);
             s->img_buffer += n;
@@ -325,9 +373,11 @@ static uint32_t *rtga_tga_load(rtga_context *s,
                   | ((uint32_t)g << 8)  | (uint32_t)b;
 
          /* Write to correct position using tracked row/col
-          * (avoids per-pixel division and modulo) */
+          * (avoids per-pixel division and modulo).  Use size_t for
+          * the index so dst_row * tga_width does not overflow
+          * signed int for a legitimate 65535 x 65535 image. */
          dst_row = tga_inverted ? (tga_height - 1 - cur_row) : cur_row;
-         output[dst_row * tga_width + cur_col] = pixel;
+         output[(size_t)dst_row * (size_t)tga_width + (size_t)cur_col] = pixel;
 
          if (++cur_col >= tga_width)
          {
@@ -366,6 +416,15 @@ int rtga_process_image(rtga_t *rtga, void **buf_data,
    if (!rtga)
       return IMAGE_PROCESS_ERROR;
 
+   /* Reject sizes that don't fit in int before casting.  A TGA
+    * file larger than INT_MAX handed to an int-taking API would
+    * truncate, the truncated value (potentially negative)
+    * propagated to img_buffer_end = buffer + len, producing
+    * pointer arithmetic outside the source object (UB).  A 2 GiB
+    * TGA is unreasonable; reject early. */
+   if (size > (size_t)INT_MAX)
+      return IMAGE_PROCESS_ERROR;
+
    rtga->output_image   = rtga_load_from_memory(rtga->buff_data,
                            (int)size, width, height, &comp, supports_rgba);
    *buf_data             = rtga->output_image;

libretro-common/samples/formats/bmp/Makefile

@@ -0,0 +1,24 @@
+TARGET := rbmp_test
+
+LIBRETRO_COMM_DIR := ../../..
+
+SOURCES := \
+	rbmp_test.c \
+	$(LIBRETRO_COMM_DIR)/formats/bmp/rbmp.c
+
+OBJS := $(SOURCES:.c=.o)
+
+CFLAGS += -Wall -pedantic -std=gnu99 -g -I$(LIBRETRO_COMM_DIR)/include
+
+all: $(TARGET)
+
+%.o: %.c
+	$(CC) -c -o $@ $< $(CFLAGS)
+
+$(TARGET): $(OBJS)
+	$(CC) -o $@ $^ $(LDFLAGS)
+
+clean:
+	rm -f $(TARGET) $(OBJS)
+
+.PHONY: clean