libretro-common: bound disc-image metadata parsers against fixed-size buffers

Two unrelated stack/heap overflows in disc-image format
parsers, both reachable when the user loads a malicious
.iso/.bin/.cue/.chd.  Same threat class as the BSV file-load
bugs.

cdfs_find_file (libretro-common/formats/cdfs/cdfs.c)
walks ECMA-119 directory records read into a 2048-byte
stack buffer.  The loop guard only checked that the
record header byte was in bounds; tmp[33 + path_length],
tmp[2..4], and tmp[10..13] could read past the buffer
end, leaking adjacent stack bytes into ';' / '\0'
comparisons and feeding attacker-influenced data into
`sector` (redirecting subsequent reads) and `file->size`.
The advance tmp += tmp[0] with attacker-controlled tmp[0]
compounded this by jumping the iterator forward arbitrarily
when records claimed lengths shorter than the legal
minimum (33 + filename + 1).  Fix: require the entire
record window to fit inside the buffer in the loop guard,
and reject records whose claimed length is below the
filename-comparison minimum.

chdstream_get_meta (libretro-common/streams/chd_stream.c)
parsed GDROM metadata KEY:VALUE pairs read into a 256-byte
stack buffer.  The TYPE: / SUBTYPE: / PGTYPE: / PGSUB:
branches all did:

  while (p[len] && p[len] != ' ') len++;
  memcpy(md->FIELD, p, len);

with no upper bound on len -- the only terminator was the
meta-buffer NUL.  A malicious .chd with "TYPE:" + 200
non-space bytes stack-overflowed md->type[64] by 136 bytes,
clobbering subtype/pgtype/pgsub and (depending on layout)
the saved frame pointer.  Fix: apply the same
"len >= sizeof(md->FIELD)" reject already used by the
SCD-format parser earlier in the same function, to all
four GDROM fields.

Adds two regression tests under libretro-common/samples,
registered in .github/workflows/Linux-libretro-common-
samples.yml's RUN_TARGETS, with ASan + UBSan made the
default for that workflow (MAKE_ARGS env var) since both
tests rely on ASan as the bound-check discriminator.
Verified locally that all 22 existing RUN_TARGETS still
pass under the new sanitizer build.

Tests follow the existing samples pattern (verbatim copy
of the post-fix predicate with maintenance-contract
comment).  Verified to fire under ASan when the bounds are
removed: heap-buffer-overflow READ at offset 2048 of a
2048-byte allocation (cdfs); heap-buffer-overflow WRITE of
size 200 at offset 180 of a 180-byte allocation (chd).
Both pass clean with the bounds in place.

Author: LibretroAdmin

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

@@ -33,6 +33,18 @@ jobs:
       - name: Build and run samples
         shell: bash
         working-directory: libretro-common/samples
+        env:
+          # Build samples under AddressSanitizer + UndefinedBehaviorSanitizer.
+          # Most overflow tests under libretro-common/samples (rpng_chunk_
+          # overflow_test, rbmp_test, vfs_read_overflow_test, cdrom_cuesheet
+          # _overflow_test, cdfs_dir_record_test, chd_meta_overflow_test, ...)
+          # use ASan as the regression discriminator for the bound-check
+          # they verify, so make ASan the default for this workflow.  Each
+          # test's Makefile honours SANITIZER= via the conventional opt-in
+          # block (matching samples/tasks/http/Makefile).  Test sources
+          # that don't have the opt-in block treat the variable as
+          # ignored.
+          MAKE_ARGS: "SANITIZER=address,undefined"
         run: |
           set -u
           set -o pipefail
@@ -55,6 +67,8 @@ jobs:
             net_ifinfo
             vfs_read_overflow_test
             cdrom_cuesheet_overflow_test
+            cdfs_dir_record_test
+            chd_meta_overflow_test
             http_parse_test
             rjson_test
             rtga_test
@@ -115,7 +129,7 @@ jobs:
             fi
 
             # Build
-            if ! ( cd "$d" && make clean all ); then
+            if ! ( cd "$d" && make clean all $MAKE_ARGS ); then
               printf '\n::error title=Build failed::%s failed to build\n' "$rel"
               fails=$((fails+1))
               continue

libretro-common/formats/cdfs/cdfs.c

@@ -185,13 +185,55 @@ static int cdfs_find_file(cdfs_file_t* file, const char* path)
    path_length = strlen(path);
    tmp         = buffer;
 
-   while (tmp < buffer + sizeof(buffer))
+   /* The directory record layout (ECMA-119 §9.1) is:
+    *   byte  0:  record length (1 byte; 0 = end of records)
+    *   byte  1:  extended-attr length
+    *   bytes 2-9: location-of-extent (LE+BE) -- we use bytes 2..4
+    *   bytes 10-17: data length (LE+BE) -- we use bytes 10..13
+    *   ...
+    *   byte 32:  filename length
+    *   bytes 33..32+filename_length: filename
+    *
+    * Pre-this-patch the read at tmp[33 + path_length] and the
+    * subsequent reads at tmp[2..4]/tmp[10..13] could run off the
+    * end of the 2048-byte stack buffer when a malformed disc
+    * image had a record positioned (or chained via the
+    * tmp += tmp[0] advance) so that tmp + 33 + path_length lay
+    * past buffer + sizeof(buffer).  An attacker who can place a
+    * crafted sector at the directory offset gets:
+    *  - a 1-byte info-leak from adjacent stack via the
+    *    tmp[33 + path_length] comparison vs ';' / '\0';
+    *  - filename-length bytes leaked via strncasecmp's
+    *    short-circuit timing;
+    *  - up to 4 bytes of attacker-influenced stack data feeding
+    *    `sector` (tmp[2..4]) and `file->size` (tmp[10..13]),
+    *    redirecting the next intfstream_read.
+    *
+    * The advance "tmp += tmp[0]" itself can land tmp anywhere
+    * up to 255 bytes ahead.  The guard tmp < buffer + sizeof
+    * only catches the case where the loop body re-runs; it does
+    * not protect against a single iteration whose body reads
+    * past the buffer.
+    *
+    * Tighten the guard so the entire record (header through the
+    * byte at offset 33 + path_length) must fit, and require the
+    * record's claimed length to be at least large enough to
+    * cover the filename comparison. */
+   while (   tmp < buffer + sizeof(buffer)
+          && (size_t)(tmp - buffer) + 33 + path_length < sizeof(buffer))
    {
       /* The first byte of the record is the length of
        * the record - if 0, we reached the end of the data */
       if (!*tmp)
          break;
 
+      /* Reject records whose claimed length cannot accommodate
+       * the filename comparison below.  A legitimate ECMA-119
+       * directory record has length >= 33 + filename_length
+       * + 1 (the version-separator byte). */
+      if (tmp[0] < 33 + path_length + 1)
+         break;
+
       /* filename is 33 bytes into the record and
        * the format is "FILENAME;version" or "DIRECTORY" */
       if (        (tmp[33 + path_length] == ';'

libretro-common/samples/formats/cdfs/Makefile

@@ -0,0 +1,25 @@
+TARGET := cdfs_dir_record_test
+
+SOURCES := cdfs_dir_record_test.c
+
+OBJS := $(SOURCES:.c=.o)
+
+CFLAGS += -Wall -pedantic -std=gnu99 -g -O0
+
+ifneq ($(SANITIZER),)
+   CFLAGS  := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CFLAGS)
+   LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
+endif
+
+all: $(TARGET)
+
+%.o: %.c
+	$(CC) -c -o $@ $< $(CFLAGS)
+
+$(TARGET): $(OBJS)
+	$(CC) -o $@ $^ $(LDFLAGS)
+
+clean:
+	rm -f $(TARGET) $(OBJS)
+
+.PHONY: clean

libretro-common/samples/formats/cdfs/cdfs_dir_record_test.c

@@ -0,0 +1,271 @@
+/* Copyright  (C) 2010-2026 The RetroArch team
+ *
+ * ---------------------------------------------------------------------------------------
+ * The following license statement only applies to this file (cdfs_dir_record_test.c).
+ * ---------------------------------------------------------------------------------------
+ *
+ * Permission is hereby granted, free of charge,
+ * to any person obtaining a copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation the rights to
+ * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+ * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
+ * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/* Regression test for the directory-record iterator bounds in
+ * libretro-common/formats/cdfs/cdfs.c::cdfs_find_file.
+ *
+ * The function reads a 2048-byte ECMA-119 directory sector
+ * from the disc image into a stack buffer and walks the
+ * variable-length records:
+ *
+ *   while (tmp < buffer + sizeof(buffer))
+ *   {
+ *      if (!*tmp) break;
+ *      if (tmp[33 + path_length] == ';' || tmp[33 + path_length] == '\0')
+ *         ...
+ *      tmp += tmp[0];
+ *   }
+ *
+ * Pre-this-patch the loop had three composable issues:
+ *
+ *  1. The guard tmp < buffer + sizeof(buffer) only confirmed
+ *     the record header byte was in bounds.  tmp[33 +
+ *     path_length] could read past the buffer end, leaking
+ *     up to ~32 bytes of adjacent stack into the comparison.
+ *
+ *  2. tmp[2..4] and tmp[10..13] were read after a successful
+ *     filename comparison, with the same shape problem -- a
+ *     record positioned near the buffer end produced
+ *     attacker-influenced stack bytes flowing into `sector`
+ *     (which redirects the next intfstream_read) and
+ *     `file->size`.
+ *
+ *  3. The advance "tmp += tmp[0]" with tmp[0] attacker-
+ *     controlled could land tmp anywhere up to 255 bytes
+ *     past the safe range.  A record that claimed length
+ *     less than 33 + filename_length + 1 (the minimum legal
+ *     ECMA-119 record size) was a primitive for jumping the
+ *     iterator to an unsafe offset.
+ *
+ * Reachability: user loads a malicious .iso/.bin/.cue.  Same
+ * threat class as other disc-image bugs (CHD, BSV).
+ *
+ * Fix: tighten the loop guard to require the entire record
+ * (header through tmp[33 + path_length]) to fit, and require
+ * the record's claimed length to be >= 33 + path_length + 1
+ * before doing the filename comparison.
+ *
+ * IMPORTANT: this test keeps a verbatim copy of the post-fix
+ * loop predicate.  If cdfs.c's loop amends, the copy below
+ * must follow.  Convention used by the other regression
+ * tests under libretro-common/samples/.
+ */
+
+#include <stdio.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <string.h>
+#include <strings.h>   /* strncasecmp */
+
+#define SECTOR_SIZE 2048
+
+static int failures = 0;
+
+/* === verbatim copy of the post-fix iterator from
+ *     libretro-common/formats/cdfs/cdfs.c::cdfs_find_file.
+ *     If cdfs.c amends, this copy must follow. ===
+ *
+ * Returns the matched record's "sector" value (tmp[2..4])
+ * on success, -1 if no record matches, -2 if the loop
+ * would have run off the buffer. */
+static int find_dir_record(const uint8_t *buffer,
+      size_t buffer_size, const char *path)
+{
+   const uint8_t *tmp = buffer;
+   size_t path_length = strlen(path);
+
+   while (   tmp < buffer + buffer_size
+          && (size_t)(tmp - buffer) + 33 + path_length < buffer_size)
+   {
+      if (!*tmp)
+         break;
+
+      if (tmp[0] < 33 + path_length + 1)
+         break;
+
+      if (        (tmp[33 + path_length] == ';'
+               || (tmp[33 + path_length] == '\0'))
+               &&  strncasecmp((const char*)(tmp + 33), path, path_length) == 0)
+      {
+         return tmp[2] | (tmp[3] << 8) | (tmp[4] << 16);
+      }
+
+      tmp += tmp[0];
+   }
+
+   return -1;
+}
+/* === end verbatim copy === */
+
+/* Build a single legitimate directory record into `out`.
+ * Returns the record's length. */
+static size_t build_record(uint8_t *out, const char *filename,
+      uint32_t sector, uint32_t size)
+{
+   size_t fn_len    = strlen(filename);
+   size_t total     = 33 + fn_len + 1;  /* +1 for ';' */
+   memset(out, 0, total);
+   out[0]           = (uint8_t)total;
+   /* sector at bytes 2..4 (little-endian 24-bit) */
+   out[2]           = (uint8_t)(sector & 0xff);
+   out[3]           = (uint8_t)((sector >> 8) & 0xff);
+   out[4]           = (uint8_t)((sector >> 16) & 0xff);
+   /* size at bytes 10..13 */
+   out[10]          = (uint8_t)(size & 0xff);
+   out[11]          = (uint8_t)((size >> 8) & 0xff);
+   out[12]          = (uint8_t)((size >> 16) & 0xff);
+   out[13]          = (uint8_t)((size >> 24) & 0xff);
+   memcpy(out + 33, filename, fn_len);
+   out[33 + fn_len] = ';';
+   return total;
+}
+
+/* ---- tests ---- */
+
+static void test_finds_legitimate_record(void)
+{
+   uint8_t buffer[SECTOR_SIZE];
+   size_t  off;
+   int     rv;
+   memset(buffer, 0, sizeof(buffer));
+   off = build_record(buffer, "FOO", 0x123456, 1024);
+   /* terminator: a 0-length record marks end-of-records */
+   buffer[off] = 0;
+
+   rv = find_dir_record(buffer, sizeof(buffer), "FOO");
+   if (rv != 0x123456)
+   {
+      printf("[ERROR] legitimate record: rv=0x%x, want 0x123456\n", rv);
+      failures++;
+   }
+   else
+      printf("[SUCCESS] legitimate record found and decoded correctly\n");
+}
+
+static void test_record_at_buffer_end_does_not_oob(void)
+{
+   /* Fill the buffer with single-byte-length records so the
+    * iterator walks 1 byte at a time, then place a non-zero
+    * byte close to the end so the guard would have allowed
+    * the body to run pre-fix.  Allocate the buffer as exactly
+    * SECTOR_SIZE bytes on the heap with no slack so ASan
+    * flags any read past the end. */
+   uint8_t *buffer = (uint8_t*)malloc(SECTOR_SIZE);
+   int rv;
+   if (!buffer) { printf("[ERROR] alloc\n"); failures++; return; }
+
+   /* Records of length 1 from offset 0 to SECTOR_SIZE - 5. */
+   memset(buffer, 1, SECTOR_SIZE - 4);
+   /* The last 4 bytes are zero, terminating the chain. */
+   memset(buffer + SECTOR_SIZE - 4, 0, 4);
+
+   /* Search for a 0-length filename: path_length = 0, so the
+    * pre-fix bug fires when tmp + 33 > buffer + 2048, i.e.
+    * tmp > buffer + 2015.  The iterator walks single-byte
+    * records right up to the boundary; pre-fix it would have
+    * read tmp[33] for tmp >= buffer + 2016. */
+   rv = find_dir_record(buffer, SECTOR_SIZE, "");
+   /* Behavioural check: no match should be found, and (under
+    * ASan) no OOB read should have occurred. */
+   if (rv >= 0)
+   {
+      /* "" matches if tmp[33] == ';' or '\0'.  Some matches are
+       * possible from random buffer content -- accept any rv,
+       * the point of the test is that ASan didn't fire. */
+   }
+   printf("[SUCCESS] record-near-buffer-end iteration stayed in bounds\n");
+
+   free(buffer);
+}
+
+static void test_short_record_length_rejected(void)
+{
+   /* A record claiming length 5 (less than the 33-byte
+    * minimum) used to advance the iterator only 5 bytes,
+    * letting subsequent reads of tmp[33+plen] reach into
+    * undefined territory.  The post-fix guard
+    *    if (tmp[0] < 33 + path_length + 1) break;
+    * stops the iterator. */
+   uint8_t *buffer = (uint8_t*)malloc(SECTOR_SIZE);
+   int rv;
+   if (!buffer) { printf("[ERROR] alloc\n"); failures++; return; }
+
+   memset(buffer, 0, SECTOR_SIZE);
+   buffer[0] = 5;  /* claimed length way below 33 */
+   /* Subsequent bytes are zero so even if the iterator
+    * advances we hit the !*tmp break. */
+
+   rv = find_dir_record(buffer, SECTOR_SIZE, "FOO");
+   if (rv != -1)
+   {
+      printf("[ERROR] short-length record produced rv=%d\n", rv);
+      failures++;
+   }
+   else
+      printf("[SUCCESS] short-length record rejected without iteration\n");
+
+   free(buffer);
+}
+
+static void test_pathological_record_at_2046(void)
+{
+   /* Place a record claiming length 1 at buffer[2046].  Pre-fix
+    * the loop guard tmp < buffer + 2048 admitted this iteration
+    * but tmp[33 + path_length] = buffer[2079 + plen] read way
+    * off the end.  Heap-alloc with exact SECTOR_SIZE so ASan
+    * catches the read.
+    *
+    * To get there the iterator needs to walk to offset 2046.
+    * Single-byte records all the way is the simplest path. */
+   uint8_t *buffer = (uint8_t*)malloc(SECTOR_SIZE);
+   int rv;
+   if (!buffer) { printf("[ERROR] alloc\n"); failures++; return; }
+
+   memset(buffer, 1, SECTOR_SIZE);
+   /* Make sure the byte at 2046 is non-zero (it already is from
+    * the memset). */
+   buffer[2046] = 1;
+   buffer[2047] = 0;  /* any value */
+
+   rv = find_dir_record(buffer, SECTOR_SIZE, "BAR");
+   /* Behaviour: no match.  Under ASan: no OOB. */
+   (void)rv;
+   printf("[SUCCESS] iterator at offset 2046 stayed in bounds\n");
+
+   free(buffer);
+}
+
+int main(void)
+{
+   test_finds_legitimate_record();
+   test_record_at_buffer_end_does_not_oob();
+   test_short_record_length_rejected();
+   test_pathological_record_at_2046();
+
+   if (failures)
+   {
+      printf("\n%d cdfs_dir_record test(s) failed\n", failures);
+      return 1;
+   }
+   printf("\nAll cdfs_dir_record regression tests passed.\n");
+   return 0;
+}