1185db89c1
Squashed series of ~50 distinct bugs found during a multi-day
security/correctness audit, ranging from ROM-triggerable heap
corruption and savestate-triggered OOB read primitives down to
obscure cross-platform savestate breakage.
Build is clean on `make platform=unix` with zero new warnings.
CRITICAL (ROM-triggerable, exploitable on the user's machine)
=============================================================
* ines.c iNES 2.0 PRG/CHR exponent overflow leading to undersized
allocation followed by heap buffer overflow on fread (pow() with
attacker-chosen exponent up to 63). Cap exponent at 30 and
compute size in uint32 with explicit cap below 2 GiB before
storing in int.
* ines.c miscROMSize int wraparound from attacker-controlled PRG/
CHR sizes; previous '& 0x8000000' check missed common underflow
cases. Compute in int64 and reject <= 0 or > 128 MiB.
* unif.c MAPR chunk OOB heap read on chunks of size < 4. Validate
chunk size before allocating board-name buffer.
* unif.c chunk size truncation: int conversion + missing cap allowed
a 4 GiB chunk size to wrap. Cap at 16 MiB.
* unif.c FixRomSize infinite loop on size > 0x80000000. Cap input.
* unif.c DINF chunk: months[(m - 1) % 12] is UB for m==0; m comes
from the .unf file. Clamp m into [1..12] before subtracting.
* unif.c NAME chunk: GameInfo->name = malloc(...) was unchecked.
* nsf.c size underflow: FCEU_fgetsize() - 0x80 wraps to ~UINT64_MAX
on tiny files, propagating a huge size into uppow2/FCEU_malloc.
Validate size > 0x80 before subtracting.
* nsf.c NSFMaxBank * 4096 cap tightened from 1<<20 (UB on signed-int
overflow) to 1<<19 (fits in int).
* general.c uppow2 had signed-int UB (1 << 32) and wrapped to 0 for
inputs near uint32 max. Cap at 0x80000000.
* cheat.c SubCheats[256] BSS overflow when more than 256 GG/PAR
substitution cheats are active. The array is adjacent to MMapPtrs[64]
in BSS which is exposed via retro_get_memory_data, making this
overflow visible from outside the core.
* libretro.c retro_cheat_set strcpy stack overflow with arbitrary-
length cheat strings from the frontend. Use strlcpy.
CRITICAL (savestate-triggerable on a malicious .fcs file)
=========================================================
* fds.c FDS savestate OOB read primitive: InDisk loaded from
savestate is used as index into 8-element diskdata[] static
pointer array. A value 8..254 dereferences arbitrary memory as
a pointer (heap-read primitive). Also bounds-checked SelectDisk,
mapperFDS_block, mapperFDS_blockstart, mapperFDS_diskaddr,
mapperFDS_blocklen so blockstart+diskaddr stays within the 65500-
byte disk buffer.
* 11 mappers had savestate-loaded variables masked at write time
but not at restore time, used as indices into fixed arrays:
- 88.c, KS7037.c, 112.c cmd indexes reg[8]
- sachen.c (S8259, S74LS374N) cmd indexes latch[8]
- 357.c dipswitch indexes outer_bank[4]
- unrom512.c flash_state indexes erase_a/d/b[5]
- 368.c preg indexes banks[8]
- 69.c sndcmd indexes sreg[14]
- mmc2and4.c latch0/latch1 index creg[4]
None individually exploitable into RCE - the array writes corrupt
adjacent BSS with constrained data flowing in - but each is an
out-of-bounds read or write from attacker-controllable input.
HIGH (memory-safety, reachable on any load)
===========================================
* fceu-memory.c FCEU_malloc deref-of-NULL on allocation failure
('ret = 0; memset(ret, 0, size);').
* file.c multiple FCEUFILE/MakeMemWrap/MakeMemWrapBuffer unchecked
allocations and unchecked filestream_tell return.
* libretro.c GameInfo NULL-derefs in three entry points
(retro_set_controller_port_device, retro_get_memory_data,
retro_get_memory_size) reachable on operations called before a
successful load.
* libretro.c framebuffer leak ~256 KB per failed FCEUI_LoadGame
(the libretro frontend doesn't call retro_unload_game on a
failed load).
* libretro.c 3DS retro_deinit unchecked linearFree.
* libretro.c stereo / NTSC filter unchecked malloc returns.
* fceu.c FCEUI_LoadGame unchecked GameInfo malloc.
* fds.c SubLoad and FDSLoad unchecked FCEU_malloc on diskdatao
backup buffers (NULL-deref in subsequent memcpy).
* fceu.c AllocGenieRW partial-failure leak: AReadG allocated but
BWriteG malloc fails -> 256 KB leak per retry.
* input/bworld.c Update(): unbounded strcpy from attacker-supplied
data into 20-byte bdata. Replaced with length-bounded copy.
* cart.c setprg2r/4r and setchr1r/2r/4r/8r mask-underflow guards.
SetupCartPRGMapping/SetupCartCHRMapping compute (size >> N) - 1
which underflows to 0xFFFFFFFF for chips smaller than the unit.
setprg8r/16r/32r and setchr8r already had defensive size checks;
the smaller units did not. malee.c (2 KB chip) and mapper 218
(2 KB NTARAM-as-CHR) accidentally avoid OOB; the primitive is
unsafe for any future board.
* video.c FCEU_InitVirtualVideo XBuf/XDBuf partial-failure leak.
* video.c, fceu.c FCEU_DispMessage / FCEU_printf / FCEU_PrintError:
vsprintf into fixed stack buffers replaced with vsnprintf.
* core: validate magic-string read length on FDS/UNIF/NSF load
(reject files too short to contain the magic so previous static
buffer / stack garbage doesn't spuriously match).
LEAKS (every cart load/unload cycle)
====================================
* mmc5.c MMC5 cart-side: WRAM (up to 64K) + MMC5fill (1K) + ExRAM
(1K) leaked per cycle. NSFMMC5_Close existed but was only used
for NSF code path.
* onebus.c Mapper 270/436: 8 KB CHRRAM leaked per cycle.
* 330.c, 375.c, 528.c: 8 KB WRAM each, no Close at all.
CORRECTNESS (UB / ABI / endianness)
===================================
* fceu-endian.c FlipByteOrder loop bound was 'count' instead of
'count/2', making it a no-op for every even count and silently
breaking savestate portability for every FCEUSTATE_RLSB-marked
field. The '#ifndef GEKKO' workarounds scattered through the
codebase (sound.c, vrc7.c, vrc6.c) were symptoms of this root
cause.
* state.c AddExState bounds check ran AFTER writing the entry, so
when SFEXINDEX hit the 63-entry cap the next call would write
the entry then immediately overwrite it with the terminator.
* state.c FCEUSS_Save_Mem: post-save callback was guarded by
'if (SPreSave)' instead of 'if (SPostSave)'.
* Sequence-point UB in counter expressions across 4 board files,
e.g. 'x = !x ? a : --x' and 'x = ++x % n'. GCC -Wsequence-point
flags all five sites (285.c, 413.c, asic_mmc3.c, jyasic.c).
* SFORMAT size mismatches between declared variable type and
AddExState size argument:
- asic_vrc3.c VRC3_count/VRC3_reload (uint16, saved as 1 byte)
- mmc3.c m555_count (uint32, saved as 2 bytes), m555_count_-
expired (uint8, saved as 2 bytes - OOB write into adjacent
BSS).
* unrom512.c UNROM-512 mapper 30 .srm save format: host-endian
uint32 flash write counters. Now stored as LE on disk regardless
of host (Battle Kid 2, Twin Dragons, Lizard, Sole, etc.).
* ~70 multi-byte single-variable SFORMAT/AddExState entries across
36 board files were saved as host-byte-order. After the
FlipByteOrder fix, these are now byte-swapped on BE so cross-
platform savestates round-trip correctly.
* coolgirl.c new ExStateLE() macro added; 22 multi-byte sites.
* pic16c5x.c 11 multi-byte AddExState calls (m_PC, m_PREVPC,
m_CONFIG, m_WDT, m_prescaler, m_opcode, m_STACK[0/1], m_icount,
m_delay_timer, m_rtcc, m_inst_cycles, m_clock2cycle).
* onebus.c PowerJoy Supermax submapper detection used non-portable
*(uint32*)&info->MD5 cast that read different bytes on LE vs BE.
* fds.c clean up redundant FCEUSTATE_RLSB encoding in AddExState
calls that also passed type=1 (idempotent OR; readability fix).
DOCUMENTATION
=============
* input.c UpdateGP's *(uint32*)data cast looks like a typical
endian bug but is actually correct (the libretro frontend builds
JSReturn with matching host-uint32 shifts). Comment added to
prevent future "fixes" from breaking it.
Limitations not addressed
=========================
* Element-stride-aware byte swapping. The savestate byte-swap
mechanism (FlipByteOrder over the entire SFORMAT entry buffer)
is structurally wrong for arrays of multi-byte values: it
reverses the whole buffer end-to-end instead of byte-swapping
each element. Several places that need cross-platform-portable
arrays (VRC7 sound state, jyasic chr[8]) work around this by
either splitting arrays into per-element SFORMAT entries (n106
PlayIndex, bandai reg) or by skipping save entirely on BE via
#ifndef GEKKO. A proper fix would extend the size encoding with
an element-stride field. Left for a future change because it
would change the savestate format.
* Strict-aliasing UB in ppu.c (around 9 sites doing
*(uint32*)uint8_buf for fast 4-byte writes via FCEU_dwmemset).
Works in practice with all common compilers because the patterns
are byte-symmetric, but is formally UB.
* FCEU_gmalloc calls exit(1) on OOM. A libretro core should never
exit() because that takes down the whole frontend. Used by
100+ call sites; refactoring to return-NULL is out of scope here.
Testing
=======
* Build: clean on `make platform=unix` with -O2; no new warnings.
* FlipByteOrder fix verified by hand-trace and a standalone unit
test for counts 2, 4, 8.
* uppow2 fix verified by unit test across 13 boundary cases.
* SFORMAT size mismatches and missing-RLSB cases identified by
Python static-analysis scripts that cross-reference SFORMAT
entries against variable declarations.
* iNES 2.0 exponent fix verified by hand-tracing what byte 0xFF
produces post-fix: exp=30 (capped), mult=7, size=3 GiB nominal,
capped to 1 GiB, capped to 2 GiB by uppow2, FCEU_malloc returns
NULL on most systems, loader returns 0. No heap overflow for
any input byte.
* Savestate-loaded array index audit: built a Python scanner that
extracts each AddExState/SFORMAT entry and cross-references the
variable name against array-index uses in the same file. All
flagged sites covered.
* A libFuzzer harness and seed corpus generator (fuzz_main.c,
gen_seed_corpus.py) accompany this submission for ongoing
regression testing.
FCE Ultra mappers modified
FCEU "mappers modified" is an unofficial build of FCEU Ultra by CaH4e3, which supports a lot of new mappers including some obscure mappers such as one for unlicensed NES ROM's.
Sequential targets Light Guns support added
Support for Sequential targets Light Guns has been added. "Gun Aux A" serves as light sensor logic input.