Security: heap- and stack-overflow fixes across image, audio, IO and cheevos paths

Fixes eight memory-safety bugs sharing a small set of root causes:
uint32_t multiplications feeding size_t allocations, ftell() and
filestream_get_size() returning -1 silently coerced to size_t,
attacker-controlled filenames used as filesystem paths, and
per-channel-count assumptions in audio decode.

* rjpeg, rpng (formats/jpeg/rjpeg.c, formats/png/rpng.c)
  Width*height*sizeof(uint32_t) wraps in uint32 on 32-bit hosts
  (3DS, Vita, PSP, Wii, Wii U, older Android, 32-bit Windows),
  the malloc returns an undersized buffer, and the per-row decode
  writes multi-GiB off the end.  Add (size_t) casts at every
  pixel-buffer malloc site so the multiplication is overflow-safe
  on 64-bit.  Add a 0x4000 dimension cap matching rbmp.c, rtga.c
  and rwebp.c, gated on 32-bit only -- on 64-bit the size_t
  arithmetic plus the existing 4 GiB output guard in
  rpng_iterate_image are sufficient, and a hard cap would reject
  legitimate large images (IrfanView and friends routinely open
  tens-of-thousands-pixel JPEGs and PNGs).

* image_transfer Gekko path (formats/image_transfer.c)
  Same uint32 multiplication primitive in the post-decode tile
  conversion on Wii.  Add (size_t) casts.  The 32-bit rpng/rjpeg
  cap closes the primitive at the source on Wii, but the casts
  are kept for defence in depth.

* audio_mixer FLAC (audio/audio_mixer.c)
  audio_mixer_mix_flac asks drflac for AUDIO_MIXER_TEMP_BUFFER/2
  frames into a 32 KiB stack buffer.  drflac writes
  frame_count*channel_count floats, so 8-channel FLAC writes
  128 KiB into the 32 KiB buffer -- a 96 KiB stack overflow
  reaching saved-RIP territory.  Reject non-stereo FLAC at
  audio_mixer_play_flac time.  Mono was already producing wrong
  audio per the existing comment in mix_flac; a proper per-
  channel-aware fix is left for a separate change.

* cheevos badge_name (cheevos/cheevos_client.c)
  rcheevos_client_download_badge interpolates the server-supplied
  badge_name into a filesystem path and writes the HTTP body
  there.  A malicious or MITM'd retroachievements.org could send
  badge_name = "../../../../etc/cron.d/evil" and write attacker-
  controlled bytes (with a forced .png suffix) anywhere on disk.
  Real badge names are numeric IDs optionally suffixed with
  "_lock"; reject anything outside [A-Za-z0-9_-].

* cheevos JSON-override loader (cheevos/cheevos_client.c)
  ftell()'s long return was assigned to size_t _len, so an ftell
  error (-1) became SIZE_MAX; malloc(_len + 1) wrapped to
  malloc(0) and contents[_len] = 0 corrupted the heap.  Capture
  into long, check for the negative sentinel.  Debug-only path
  (CHEEVOS_JSON_OVERRIDE).

* rxml (formats/xml/rxml.c)
  Same pattern, file-scale: filestream_get_size returns -1 on
  error and silently flowed into (size_t)(len + 1) as 0 on 64-bit
  (malloc(0) -> tiny block) or as a wrapped value on 32-bit
  (undersized buffer).  Either way memory_buffer[len] = '\0'
  wrote far out of bounds.  Reject negative sizes and any size
  that wouldn't fit in size_t.  Reachable via .xml shader presets
  and Logiqx .dat database files.

* filestream_read_file (streams/file_stream.c)
  The existing size check was
    if ((int64_t)(uint64_t)(size + 1) != (size + 1)) goto error;
  -- tautological for any positive int64_t, never trips.  On
  32-bit hosts any file > ~4 GiB silently truncates through the
  size_t cast on the malloc and the subsequent filestream_read
  overruns the heap.  Replace with an explicit size_t-fit check.

* test/formats/test_rpng.c (new)
  Six libcheck cases covering rpng_process_ihdr.  The two that
  exercise the 0x4000 cap (0x4001 and 30000^2) are gated on 32-
  bit hosts only since on 64-bit those dimensions are legitimate
  after this change.  The remaining four (accept-at-limit,
  uint32-max-reject, accept-small, zero-rejected) run on every
  platform.  Wired into Makefile.test alongside the existing
  string/utils/hash/lists/queues suites; builds with -Werror
  under the same TEST_UNIT_CFLAGS the other tests use; pulls in
  trans_stream*.c + -lz for the zlib backend reference rpng.c
  links to.

Author: LibretroAdmin

cheevos/cheevos_client.c

@@ -294,6 +294,7 @@ void rcheevos_client_http_load_response(const rc_api_request_t* request,
    rc_client_server_callback_t callback, void* callback_data)
 {
    char* contents;
+   long  ftell_ret;
    size_t _len = 0;
    FILE* file  = fopen(CHEEVOS_JSON_OVERRIDE, "rb");
 
@@ -311,9 +312,23 @@ void rcheevos_client_http_load_response(const rc_api_request_t* request,
    }
 
    fseek(file, 0, SEEK_END);
-   _len = ftell(file);
+   ftell_ret = ftell(file);
    fseek(file, 0, SEEK_SET);
 
+   /* ftell can return -1 on stream error.  Pre-patch _len was
+    * declared size_t and assigned directly from ftell, so a
+    * negative return became (size_t)-1 == SIZE_MAX, malloc
+    * (SIZE_MAX + 1) wrapped to malloc(0) returning a tiny
+    * non-NULL block, and the subsequent contents[_len] = 0
+    * write at offset SIZE_MAX corrupted the heap. */
+   if (ftell_ret < 0)
+   {
+      fclose(file);
+      callback(NULL, 0, callback_data);
+      return;
+   }
+   _len = (size_t)ftell_ret;
+
    contents = (char*)malloc(_len + 1);
    /* NULL-check the malloc: without this the contents[_len] = 0
     * write below and the fread into contents NULL-deref. */
@@ -489,6 +504,39 @@ bool rcheevos_client_download_badge(rc_client_download_queue_t* queue,
    fill_pathname_slash(badge_fullpath, sizeof(badge_fullpath));
    badge_fullname      = badge_fullpath + strlen(badge_fullpath);
    badge_fullname_size = sizeof(badge_fullpath) - (badge_fullname - badge_fullpath);
+
+   /* badge_name is supplied by the achievement server (or, on a
+    * compromised TLS path, an attacker-controlled MITM).
+    * fill_pathname_slash ensures we are anchored under the
+    * badges directory, but if badge_name contains '..', '/', '\\'
+    * or other path-component separators the resulting filesystem
+    * write escapes that directory.  Validate that badge_name is
+    * a single safe filename component (alphanumerics plus '_' and
+    * '-' suffixed by '_lock' on the lock variant); the underscore
+    * is enough because real badge names from the server are
+    * numeric IDs ("12345") or numeric IDs with a "_lock" suffix.
+    * Reject anything else rather than synthesising a sanitised
+    * version, since a bogus badge name from the server is itself
+    * a signal that something is wrong. */
+   {
+      const char *p;
+      bool        ok = (badge_name && *badge_name);
+      for (p = badge_name; ok && *p; p++)
+      {
+         char c = *p;
+         if (!(   (c >= '0' && c <= '9')
+               || (c >= 'a' && c <= 'z')
+               || (c >= 'A' && c <= 'Z')
+               || c == '_' || c == '-'))
+            ok = false;
+      }
+      if (!ok)
+      {
+         CHEEVOS_LOG(RCHEEVOS_TAG "Rejecting badge with unsafe name.\n");
+         return false;
+      }
+   }
+
    snprintf(badge_fullname, badge_fullname_size, "%s" FILE_PATH_PNG_EXTENSION, badge_name);
 
    if (path_is_valid(badge_fullpath))

libretro-common/Makefile.test

@@ -23,6 +23,12 @@ TEST_HASH_SRC = test/hash/test_hash.c hash/lrc_hash.c \
 		streams/file_stream.c vfs/vfs_implementation.c file/file_path.c \
 		compat/compat_strl.c time/rtime.c string/stdstring.c encodings/encoding_utf.c
 
+TEST_RPNG = test/formats/test_rpng
+TEST_RPNG_SRC = test/formats/test_rpng.c formats/png/rpng.c \
+		streams/trans_stream.c streams/trans_stream_zlib.c \
+		streams/trans_stream_pipe.c
+TEST_RPNG_LIBS = -lz
+
 all:
 	# Build and execute tests in order, to avoid coverage file collision
 	# string
@@ -45,12 +51,17 @@ all:
 	$(CC) $(TEST_UNIT_CFLAGS) $(TEST_GENERIC_QUEUE_SRC) -o $(TEST_GENERIC_QUEUE)
 	$(TEST_GENERIC_QUEUE)
 	lcov -c -d . -o `dirname $(TEST_GENERIC_QUEUE)`/coverage.info
+	# rpng
+	$(CC) $(TEST_UNIT_CFLAGS) $(TEST_RPNG_SRC) $(TEST_RPNG_LIBS) -o $(TEST_RPNG)
+	$(TEST_RPNG)
+	lcov -c -d . -o `dirname $(TEST_RPNG)`/coverage.info
 
 	lcov -o test/coverage.info \
 	     -a test/utils/coverage.info \
 	     -a test/string/coverage.info \
 	     -a test/lists/coverage.info \
-	     -a test/queues/coverage.info
+	     -a test/queues/coverage.info \
+	     -a test/formats/coverage.info
 	genhtml -o test/coverage/ test/coverage.info
 
 clean:

libretro-common/audio/audio_mixer.c

@@ -723,6 +723,25 @@ static bool audio_mixer_play_flac(
 
    if (!dr_flac)
       return false;
+
+   /* The downstream mixer (audio_mixer_mix_flac) requests
+    * AUDIO_MIXER_TEMP_BUFFER / 2 frames into a stack buffer
+    * sized AUDIO_MIXER_TEMP_BUFFER floats.  drflac writes
+    * frame_count * channel_count floats, so this only fits
+    * exactly for stereo.  Mono fits but the downstream
+    * accounting is wrong (per existing comment); >2 channels
+    * overflows the stack buffer (e.g. 8-channel FLAC writes
+    * 4 * AUDIO_MIXER_TEMP_BUFFER floats = 4x the buffer).
+    * Reject anything that isn't stereo here rather than risk a
+    * stack overflow during mix.  Mono should be fixed
+    * separately by adjusting the mixer's per-channel
+    * accounting. */
+   if (dr_flac->channels != 2)
+   {
+      drflac_close(dr_flac);
+      return false;
+   }
+
    if (dr_flac->sampleRate != s_rate)
    {
       ratio = (double)s_rate / (double)(dr_flac->sampleRate);

libretro-common/formats/image_transfer.c

@@ -299,12 +299,26 @@ int image_transfer_process(
       unsigned tmp_pitch, width2, i;
       const uint16_t *src = NULL;
       uint16_t *dst       = NULL;
-      void *tmp           = malloc((*width) * (*height) * sizeof(uint32_t));
+      /* (size_t) casts on width and height: pre-patch the uint32
+       * multiplication width * height * 4 wrapped on 32-bit Wii
+       * (Gekko is a 32-bit PowerPC) for any image with
+       * width*height > 2^30, the malloc returned an undersized
+       * buffer, and the memcpy below ran off the end.  This file
+       * is reached only after rpng/rjpeg has already accepted the
+       * image; on 32-bit (which is where this matters) those
+       * decoders cap dimensions at 0x4000 which closes the
+       * primitive at the source.  The casts here keep the
+       * arithmetic safe regardless of upstream caps and on any
+       * platform where image_transfer.c is compiled, including
+       * future 64-bit Wii-class targets. */
+      void *tmp           = malloc(
+            (size_t)(*width) * (size_t)(*height) * sizeof(uint32_t));
 
       if (!tmp)
          return IMAGE_PROCESS_ERROR;
 
-      memcpy(tmp, *buf, (*width) * (*height) * sizeof(uint32_t));
+      memcpy(tmp, *buf,
+            (size_t)(*width) * (size_t)(*height) * sizeof(uint32_t));
       tmp_pitch = ((*width) * sizeof(uint32_t)) >> 1;
 
       *width  &= ~3;

libretro-common/formats/jpeg/rjpeg.c

@@ -2583,6 +2583,24 @@ static int rjpeg_process_frame_header(rjpeg_jpeg *z, int scan)
    if (s->img_x == 0)
       return 0;
 
+   /* On 32-bit hosts the (size_t) casts on the malloc sites below
+    * are not enough by themselves: the per-component arena and
+    * iter_output buffers can still demand multi-hundred-MB
+    * allocations that fragment or fail the address space, even
+    * with overflow-safe arithmetic.  Cap dimensions on 32-bit
+    * (matching the cap rbmp.c, rtga.c and rwebp.c apply) to
+    * preserve the previous host-resource ceiling there.
+    *
+    * On 64-bit hosts the SIZE_MAX/4 guard further down in this
+    * function plus the (size_t) casts make the allocations safe
+    * regardless of dimensions, and a desktop user loading a
+    * large legitimate JPEG (cf. IrfanView, image-pipeline tools)
+    * is a real use case.  Do not cap there. */
+#if SIZE_MAX <= 0xFFFFFFFFu
+   if (s->img_x > 0x4000u || s->img_y > 0x4000u)
+      return 0;
+#endif
+
    c = rjpeg_get8(s);
 
    /* JFIF requires */
@@ -4149,7 +4167,11 @@ static int rjpeg_iterate_init_resample(rjpeg_t *rjpeg)
          r->resample = j->resample_row_hv_2_kernel;
    }
 
-   rjpeg->iter_output = (uint8_t *)malloc(4 * j->img_x * j->img_y);
+   /* (size_t) casts: img_x and img_y are uint32_t.  Even with the
+    * 0x4000 cap above this only matters on 32-bit hosts, but the
+    * cast makes the expression robust against future cap changes. */
+   rjpeg->iter_output = (uint8_t *)malloc(
+         (size_t)4 * (size_t)j->img_x * (size_t)j->img_y);
    if (!rjpeg->iter_output)
       return 0;
 
@@ -4653,7 +4675,7 @@ int rjpeg_process_image(rjpeg_t *rjpeg, void **buf_data,
 
       /* Allocate output buffer */
       proc->output = (uint8_t *) malloc(
-            proc->n * j->img_x * j->img_y);
+            (size_t)proc->n * (size_t)j->img_x * (size_t)j->img_y);
       if (!proc->output)
       {
          rjpeg_process_free(proc);

libretro-common/formats/png/rpng.c

@@ -658,6 +658,28 @@ static bool rpng_process_ihdr(struct png_ihdr *ihdr)
          return false;
    }
 
+   /* On 32-bit hosts the per-row decode mallocs cannot fit much
+    * more than 1 GiB of decoded RGBA, and an undersized malloc
+    * combined with attacker-controlled dimensions has historically
+    * been the heap-overflow primitive prompting the 0x4000 caps
+    * in rbmp.c, rtga.c and rwebp.c.  Keep the tight cap there.
+    *
+    * On 64-bit the (size_t) casts in rpng_reverse_filter_init and
+    * the final allocator make the per-row arithmetic overflow-safe
+    * regardless of dimensions, and the 4 GiB output guard further
+    * down in rpng_iterate_image already rejects images whose
+    * decoded buffer cannot be addressed.  Loading a 30000x30000
+    * RGBA image on a desktop with the RAM to spare is a legitimate
+    * use case (cf. IrfanView), so do not impose the 0x4000 cap
+    * there. */
+#if SIZE_MAX <= 0xFFFFFFFFu
+   if (ihdr->width > 0x4000u || ihdr->height > 0x4000u)
+   {
+      fprintf(stderr, "[RPNG] Error in line %d.\n", __LINE__);
+      return false;
+   }
+#endif
+
 #ifdef RPNG_TEST
    fprintf(stderr, "IHDR: (%u x %u), bpc = %u, palette = %s, color = %s, alpha = %s, adam7 = %s.\n",
          ihdr->width, ihdr->height,
@@ -696,6 +718,15 @@ static bool rpng_process_ihdr(struct png_ihdr *ihdr)
          return false;
    }
 
+   /* See the matching comment in the RPNG_TEST/DEBUG variant
+    * above.  Cap only on 32-bit; 64-bit lets the (size_t)
+    * widening + 4 GiB output guard handle large legitimate
+    * images. */
+#if SIZE_MAX <= 0xFFFFFFFFu
+   if (ihdr->width > 0x4000u || ihdr->height > 0x4000u)
+      return false;
+#endif
+
    return true;
 }
 #endif
@@ -1063,7 +1094,7 @@ static int rpng_reverse_filter_init(const struct png_ihdr *ihdr,
             rpng_passes[pngp->pass_pos].stride_y - 1) / rpng_passes[pngp->pass_pos].stride_y;
 
       if (!(pngp->data = (uint32_t*)malloc(
-            pngp->pass_width * pngp->pass_height * sizeof(uint32_t))))
+            (size_t)pngp->pass_width * (size_t)pngp->pass_height * sizeof(uint32_t))))
          return -1;
 
       pngp->ihdr        = *ihdr;
@@ -1371,11 +1402,11 @@ static int rpng_load_image_argb_process_inflate_init(
 #ifdef GEKKO
    /* We often use these in textures, make sure 
     * they're 32-byte aligned */
-   *data = (uint32_t*)memalign(32, rpng->ihdr.width *
-         rpng->ihdr.height * sizeof(uint32_t));
+   *data = (uint32_t*)memalign(32, (size_t)rpng->ihdr.width *
+         (size_t)rpng->ihdr.height * sizeof(uint32_t));
 #else
-   *data = (uint32_t*)malloc(rpng->ihdr.width *
-         rpng->ihdr.height * sizeof(uint32_t));
+   *data = (uint32_t*)malloc((size_t)rpng->ihdr.width *
+         (size_t)rpng->ihdr.height * sizeof(uint32_t));
 #endif
    if (!*data)
       goto false_end;

libretro-common/formats/xml/rxml.c

@@ -21,6 +21,7 @@
  */
 
 #include <string.h>
+#include <stdint.h>
 
 #include <boolean.h>
 #include <streams/file_stream.h>
@@ -101,6 +102,14 @@ rxml_document_t *rxml_load_document(const char *path)
       return NULL;
 
    len                     = filestream_get_size(file);
+   /* filestream_get_size returns -1 on error.  Pre-patch this
+    * flowed through (size_t)(len + 1) as malloc(0) on 64-bit
+    * (returning a tiny non-NULL block) or as a wrapped value on
+    * 32-bit; either way memory_buffer[len] = '\0' wrote far
+    * out-of-bounds.  Reject negative sizes and any size that
+    * would not fit in size_t on this platform. */
+   if (len < 0 || (uint64_t)len >= (uint64_t)((size_t)-1))
+      goto error;
    memory_buffer           = (char*)malloc((size_t)(len + 1));
    if (!memory_buffer)
       goto error;

libretro-common/streams/file_stream.c

@@ -629,9 +629,17 @@ int64_t filestream_read_file(const char *path, void **buf, int64_t *len)
    if ((content_buf_size = filestream_get_size(file)) < 0)
       goto error;
 
-   if (!(content_buf = malloc((size_t)(content_buf_size + 1))))
+   /* Reject sizes that would not survive the cast to size_t for
+    * the malloc below.  Pre-patch the only check here was a
+    * tautological '(int64_t)(uint64_t)X != X' (which is false
+    * for any positive int64_t), and on 32-bit hosts any file
+    * larger than ~4 GiB silently truncated through (size_t),
+    * the malloc was undersized, and the filestream_read below
+    * overran it. */
+   if ((uint64_t)content_buf_size + 1 > (uint64_t)((size_t)-1))
       goto error;
-   if ((int64_t)(uint64_t)(content_buf_size + 1) != (content_buf_size + 1))
+
+   if (!(content_buf = malloc((size_t)(content_buf_size + 1))))
       goto error;
 
    if ((ret = filestream_read(file, content_buf, (int64_t)content_buf_size)) <

ui/drivers/ui_qt_widgets.cpp

@@ -7508,7 +7508,8 @@ bool MainWindow::addDirectoryFilesToList(QProgressDialog *dialog,
                          * Don't just extend this to add all files
                          * in a ZIP, because we might hit something like
                          * MAME/FBA where only the archives themselves
-                         /* Only append inner file reference if filter inside archives is enabled */
+                         * Only append inner file reference if filter 
+                         * inside archives is enabled */
                         if (playlistDialog->filterInArchive())
                         {
                             pathArray = (QString(pathData)