libretro-common/formats/png/rpng: integer overflow hardening + test

Audit of rpng.c (1859 lines; the PNG decoder used for
thumbnails, cores' asset bundles, RetroAchievement badges,
and every screenshot displayed by the menu).  Three integer-
overflow fixes plus a regression test.

rpng: replace pointer-arithmetic chunk bound check
  The chunk-size guard in rpng_iterate_image was:

    if (buf + 8 + chunk_size > rpng->buff_end)
       return false;

  where chunk_size is a 32-bit attacker-controlled value from
  the PNG stream.  On 32-bit builds a value near UINT32_MAX
  wraps the pointer address, the compare is defeated, and the
  subsequent IDAT handler's
      memcpy(rpng->idat_buf.data + rpng->idat_buf.size,
             buf, chunk_size);
  can read up to ~4 GiB past the end of the input buffer.  On
  64-bit the pointer has enough headroom that the compare
  still happens to give the right answer in practice, but the
  arithmetic is still UB per C99 (pointer values outside the
  bounds of the object, and not one-past).

  Fix: compute bytes-remaining as a size_t and compare sizes
  rather than pointers.  The new check also requires all 12
  bytes of chunk framing (length + type + CRC) -- pre-patch
  the pointer compare happened to only enforce 8 bytes'
  presence, letting a chunk whose declared length consumed
  all but 1..3 bytes of the input pass (not exploitable on
  its own because the advance at the end of the function
  then tripped the next-iteration check, but a real
  correctness bug that the size compare fixes for free).

rpng: cap IDAT accumulation at 256 MiB
  rpng_realloc_idat grew an internal buffer on each IDAT
  chunk via:

    size_t required = buf->size + chunk_size;
    if (required > buf->capacity) {
       while (new_cap < required) new_cap *= 2;
       realloc(...);
    }

  On 32-bit size_t the addition could wrap after many chunks
  accumulated, making the compare falsely false and the
  memcpy overrun the existing allocation.  The doubling loop
  could similarly overflow new_cap on its own for a single
  oversized chunk.  On 64-bit the arithmetic holds, but there
  was nothing stopping a hostile PNG from streaming in
  arbitrarily many IDAT chunks to drive the allocation up.

  Fix: introduce RPNG_IDAT_MAX = 256 MiB (dwarfs any
  realistic libretro asset) and reject both the per-chunk
  addition and the doubling loop if it would exceed that
  cap.  Accumulated total cannot grow past RPNG_IDAT_MAX.

rpng: widen buff_data advance to size_t
  At the end of rpng_iterate_image:

    rpng->buff_data += chunk_size + 12;

  chunk_size + 12 is computed in uint32_t (uint32_t + int
  promotes to uint32_t), so chunk_size near UINT32_MAX wraps.
  The per-chunk-size overflow guard above now rejects any
  value large enough to trigger this, but keep the
  arithmetic explicit in size_t so that a future relaxation
  of the guard upstream doesn't silently reintroduce the
  wrap.

=== Regression test ===

libretro-common/samples/formats/png/rpng_chunk_overflow_test.c
  Three subtests exercising the chunk-header guard:

    1. chunk_size = 0xFFFFFFF8 must be rejected
    2. chunk_size larger than remaining input rejected
    3. Valid minimal IHDR chunk iterates cleanly (smoke)

  Honest note on discriminator status: the headline bug is a
  pointer-arithmetic overflow only genuinely reachable on
  32-bit builds -- on a 64-bit host the pointer has enough
  headroom that the compare still happens to give the right
  answer (the arithmetic itself is UB, but no sanitizer
  flags it in practice).  These tests therefore exercise the
  NEW size_t-based guard and serve as regression protection
  against anyone reintroducing unguarded pointer arithmetic;
  they are not pre/post discriminators on a 64-bit CI host.
  Compiled with -m32 or run on a 32-bit host, subtest #1
  becomes a true discriminator.

  The second fix (RPNG_IDAT_MAX cap) is not directly
  testable in a CI sample -- exercising the accumulator
  overflow needs either a truly 32-bit build environment or
  a >256 MiB crafted PNG, neither of which belong in a unit
  test.  The fix is localised, the cap is explicit, and the
  code path leading to it is already exercised by the
  existing rpng encode+decode smoke test.

CI: libretro-common samples workflow updated.
rpng_chunk_overflow_test added to RUN_TARGETS.  Full local
dry-run under the GHA shell contract:
    Built: 17  Ran: 18  Failed: 0
  (Ran>Built because the PNG directory's Makefile now builds
  both the existing `rpng` interactive sample and the new
  `rpng_chunk_overflow_test` in one invocation; the build
  count is per-directory.)

VC6 compatibility: rpng.c is compiled on all platforms.  The
fix uses only size_t / uint32_t and standard comparisons;
no new headers.

Author: LibretroAdmin

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

@@ -58,6 +58,7 @@ jobs:
             rjson_test
             rtga_test
             rbmp_test
+            rpng_chunk_overflow_test
           )
 
           # Per-binary run command (overrides ./<binary> if present).

libretro-common/formats/png/rpng.c

@@ -1397,17 +1397,48 @@ static int rpng_load_image_argb_process_inflate_init(
    return -1;
 }
 
+/* Cap on the accumulated IDAT stream.  The IHDR check rejects
+ * images whose decoded output would exceed 4 GiB, so any legitimate
+ * IDAT stream will be well under this ceiling.  256 MiB of
+ * compressed IDAT is far larger than any realistic libretro asset
+ * and small enough that even on 32-bit the doubling loop below
+ * cannot overflow.  Rejecting here closes off a decompression-bomb
+ * path where a hostile PNG streams many large IDAT chunks to force
+ * the intermediate buffer to grow arbitrarily. */
+#define RPNG_IDAT_MAX ((size_t)256 * 1024 * 1024)
+
 static bool rpng_realloc_idat(struct idat_buffer *buf, uint32_t chunk_size)
 {
-   size_t required = buf->size + chunk_size;
+   size_t required;
+
+   /* Pre-patch: buf->size + chunk_size was size_t + uint32_t.  On
+    * 32-bit size_t the sum could wrap (accumulated IDAT plus a
+    * near-UINT32_MAX chunk_size), making "required > capacity"
+    * false, the realloc skipped, and the subsequent memcpy writing
+    * past the existing buffer.  Detect overflow explicitly and
+    * cap total growth. */
+   if (chunk_size > RPNG_IDAT_MAX - buf->size)
+      return false;
+   required = buf->size + chunk_size;
 
    if (required > buf->capacity)
    {
       uint8_t *new_buffer = NULL;
       size_t new_cap      = buf->capacity ? buf->capacity : 4096;
 
+      /* Cap the doubling too so a malicious chunk at the edge of
+       * RPNG_IDAT_MAX cannot drive new_cap past SIZE_MAX / 2. */
       while (new_cap < required)
+      {
+         if (new_cap > RPNG_IDAT_MAX / 2)
+         {
+            new_cap = RPNG_IDAT_MAX;
+            break;
+         }
          new_cap *= 2;
+      }
+      if (new_cap < required)
+         return false;
 
       new_buffer = (uint8_t*)realloc(buf->data, new_cap);
 
@@ -1516,20 +1547,34 @@ bool rpng_iterate_image(rpng_t *rpng)
 {
    uint8_t *buf             = (uint8_t*)rpng->buff_data;
    uint32_t chunk_size      = 0;
+   size_t   remaining;
 
    /* Check whether data buffer pointer is valid */
    if (buf > rpng->buff_end)
       return false;
 
    /* Check whether reading the header will overflow
-    * the data buffer */
-   if (rpng->buff_end - buf < 8)
+    * the data buffer.  buff_end points at the LAST byte of the
+    * input, so bytes-remaining = (buff_end - buf) + 1. */
+   if ((size_t)(rpng->buff_end - buf) + 1 < 8)
       return false;
 
    chunk_size = rpng_dword_be(buf);
 
-   /* Check whether chunk will overflow the data buffer */
-   if (buf + 8 + chunk_size > rpng->buff_end)
+   /* Check whether chunk will overflow the data buffer.
+    *
+    * Pre-patch:
+    *    if (buf + 8 + chunk_size > rpng->buff_end) return false;
+    * is pointer arithmetic on a uint8_t * with an attacker-
+    * controlled 32-bit chunk_size.  For a value near UINT32_MAX
+    * the sum wraps the pointer address (UB per C99; on 32-bit the
+    * arithmetic genuinely rolls over and the compare defeats the
+    * check, letting the memcpy at the IDAT handler read ~4 GiB
+    * past the end of the input).  Compare sizes instead of
+    * pointers, and reject chunk_size that cannot possibly fit
+    * even before accounting for the type/CRC overhead. */
+   remaining = (size_t)(rpng->buff_end - buf) + 1;
+   if (chunk_size > remaining || remaining - chunk_size < 12)
       return false;
 
    switch (rpng_read_chunk_header(buf, chunk_size))
@@ -1702,7 +1747,13 @@ bool rpng_iterate_image(rpng_t *rpng)
          return false;
    }
 
-   rpng->buff_data += chunk_size + 12;
+   /* chunk_size + 12 is a uint32_t + int, promoted to uint32_t,
+    * which wraps for chunk_size near UINT32_MAX.  The
+    * per-chunk-size overflow guard at the top of this function
+    * already rejects values that large, but keep the arithmetic
+    * explicit in size_t here so readers (and future callers who
+    * might loosen that guard) don't trip the wrap. */
+   rpng->buff_data += (size_t)chunk_size + 12;
 
    /* Check whether data buffer pointer is valid */
    if (rpng->buff_data > rpng->buff_end)

libretro-common/samples/formats/png/Makefile

@@ -1,4 +1,5 @@
 TARGET := rpng
+TARGET_TEST := rpng_chunk_overflow_test
 
 CORE_DIR          := .
 LIBRETRO_PNG_DIR  := ../../../formats/png
@@ -43,19 +44,35 @@ SOURCES_C := 	\
 	$(LIBRETRO_COMM_DIR)/time/rtime.c \
 	$(LIBRETRO_COMM_DIR)/lists/string_list.c
 
+# The chunk-overflow regression test exercises only
+# rpng_iterate_image, which doesn't invoke the inflate stream,
+# but rpng.c itself pulls in trans_stream.h so the
+# trans_stream/zlib objects must still be linked in.
+TEST_SOURCES_C := \
+	$(CORE_DIR)/rpng_chunk_overflow_test.c \
+	$(LIBRETRO_PNG_DIR)/rpng.c \
+	$(LIBRETRO_COMM_DIR)/encodings/encoding_crc32.c \
+	$(LIBRETRO_COMM_DIR)/streams/trans_stream.c \
+	$(LIBRETRO_COMM_DIR)/streams/trans_stream_zlib.c \
+	$(LIBRETRO_COMM_DIR)/streams/trans_stream_pipe.c
+
 OBJS := $(SOURCES_C:.c=.o)
+TEST_OBJS := $(TEST_SOURCES_C:.c=.o)
 
 CFLAGS += -Wall -pedantic -std=gnu99 -O0 -g -DHAVE_ZLIB -DRPNG_TEST -I$(LIBRETRO_COMM_DIR)/include
 
-all: $(TARGET)
+all: $(TARGET) $(TARGET_TEST)
 
 %.o: %.c
 	$(CC) -c -o $@ $< $(CFLAGS)
 
 $(TARGET): $(OBJS)
 	$(CC) -o $@ $^ $(LDFLAGS)
 
+$(TARGET_TEST): $(TEST_OBJS)
+	$(CC) -o $@ $^ $(LDFLAGS)
+
 clean:
-	rm -f $(TARGET) $(OBJS)
+	rm -f $(TARGET) $(TARGET_TEST) $(OBJS) $(TEST_OBJS)
 
 .PHONY: clean