mupen64plus/mupen64plus-core

Missing ROM file size validation

Closed this issue · 1 comments

269261 commented

Issue 1 - max file size validation

When ROM file is loaded, buffer with its content is passed to open_rom function, and then copied to g_mem_base buffer:

mupen64plus-core/src/main/rom.c

Line 159 in f500eb5

swap_copy_rom((uint8_t*)mem_base_u32(g_mem_base, MM_CART_ROM), romimage, size, &imagetype);

Since g_mem_base buffer has constant size, loading a malicious, big ROM (> 256MiB) will lead to buffer overflow. A sample file can be generated using the following Python script:

CONTENT_SIZE = 260 # MiB
with open("too-big.z64", "wb") as file:
    file.write(b"\x80\x37\x12\x40")
    for i in range(CONTENT_SIZE):
        file.write(b"\x00"*1024*1024)

With this file loaded, emulator will crash:

$ gdb -ex run --args Bin/Release/RMG /tmp/too-big.z64
GNU gdb (Ubuntu 13.1-2ubuntu2) 13.1
...

Thread 14 "Thread::Emulati" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffd13fb6c0 (LWP 184990)]
__memcpy_sse2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:839
839	../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file or directory.
(gdb) bt
#0  __memcpy_sse2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:839
#1  0x00007fffc805ef9e in swap_copy_rom (dst=0x7fffb7ff0000, src=0x7fff7bbff010, len=272629764, imagetype=0x7fffd13f92d3 "")
    at /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:135
#2  0x00007fffc805f060 in open_rom (romimage=0x7fff7bbff010 "\2007\022@", size=272629764) at /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:159
#3  0x00007fffc800d670 in CoreDoCommand (Command=M64CMD_ROM_OPEN, ParamInt=272629764, ParamPtr=0x7fff7bbff010) at /tmp/RMG/Source/3rdParty/mupen64plus-core/src/api/frontend.c:185
...

It seems that a simple romsize <= CART_ROM_MAX_SIZE check should fix this problem, either in open_rom function or CoreDoCommand -> M64CMD_ROM_OPEN (where minimum file size verification takes place). While the 64MB cap is disputed (e.g. #997), the max ROM file size check is still needed to prevent memory corruption.

Issue 2 - Out-of-bounds read in ROM conversion to native format

If ROM file in non-native format is loaded, its content will have 16-bit half-words (.v64) or 32-bit words (.n64) swapped by the swap_copy_rom function:

mupen64plus-core/src/main/rom.c

Lines 107 to 119 in f500eb5

if (memcmp(src, V64_SIGNATURE, sizeof(V64_SIGNATURE)) == 0)
{
size_t i;
const uint16_t* src16 = (const uint16_t*) src;
uint16_t* dst16 = (uint16_t*) dst;
*imagetype = V64IMAGE;
/* .v64 images have byte-swapped half-words (16-bit). */
for (i = 0; i < len; i += 2)
{
*dst16++ = m64p_swap16(*src16++);
}
}

mupen64plus-core/src/main/rom.c

Lines 120 to 132 in f500eb5

else if (memcmp(src, N64_SIGNATURE, sizeof(N64_SIGNATURE)) == 0)
{
size_t i;
const uint32_t* src32 = (const uint32_t*) src;
uint32_t* dst32 = (uint32_t*) dst;
*imagetype = N64IMAGE;
/* .n64 images have byte-swapped words (32-bit). */
for (i = 0; i < len; i += 4)
{
*dst32++ = m64p_swap32(*src32++);
}
}

Conversion is done under assumption that content size is divisible by 2 (.v64) or by 4 (.n64). If a file not meeting this criteria is loaded, then application will read some bytes past the ROM buffer.

Let's assume that swap_copy_rom function was called for ROM file containing only V64 header (4 bytes - minimum ROM size is 4096, but it's only to show the concept):

rombuffer = {0x37, 0x80, 0x40, 0x12}

During conversion, the following bytes will be written to the destination buffer:

{0x80, 0x37, 0x12, 0x40}

This result is correct. But what if we added another byte to initial rombuffer (ROM size = 5):

rombuffer = {0x37, 0x80, 0x40, 0x12, 0xFF}

Destination buffer will get:

{0x80, 0x37, 0x12, 0x40, 0x??, 0xFF}

What is 0x??? It's the byte in memory right after rombuffer space:

+--------+ +--------+ +--------+ +--------+ +--------+ +--------+
|        | |        | |        | |        | |        | |        |
|  0x37  | |  0x80  | |  0x40  | |  0x12  | |  0xFF  | |  0x??  |
|        | |        | |        | |        | |        | |        |
+--------+ +--------+ +--------+ +--------+ +--------+ +----+---+
                                                            ^
+                                                    +      |
+-------------------------+--------------------------+      |
                          +                                 +
                  rombuffer content               something after rombuffer

In this case:

  • 0x80 changed place with 0x37
  • 0x40 changed place with 0x12
  • 0xFF changed place with 0x??

So if V64 file with size not divisible by 2 is loaded, then the application will read 1 byte past the ROM buffer. Same issue exists for N64 files, but in this case application could read up to 3 bytes past the ROM buffer (since 32-bit values are swapped for .n64 files).

Sample files can be generated using the following Python script:

FILE_SIZE = 4099
with open("size-not-divisible-by-2.v64", "wb") as file:
    file.write(b"\x37\x80\x40\x12")
    file.write(b"\x00" * (FILE_SIZE-4))
    assert(file.tell() % 2 != 0)
with open("size-not-divisible-by-4.n64", "wb") as file:
    file.write(b"\x40\x12\x37\x80")
    file.write(b"\x00" * (FILE_SIZE-4))
    assert(file.tell() % 4 != 0)

With one of these files loaded, emulator will crash if mupen64plus-core was compiled with AddressSanitizer (otherwise no crash will happen).

Result for generated .v64 file:

$ ./BinAsan/Release/RMG /tmp/size-not-divisible-by-2.v64
=================================================================
==185613==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6210002e0102 at pc 0x7ff9edef7c82 bp 0x7ff9c3c9e0e0 sp 0x7ff9c3c9e0d0
READ of size 2 at 0x6210002e0102 thread T13 (Thread::Emulati)
    #0 0x7ff9edef7c81 in swap_copy_rom /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:117
    #1 0x7ff9edef7fd9 in open_rom /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:159
    #2 0x7ff9ede31a9f in CoreDoCommand /tmp/RMG/Source/3rdParty/mupen64plus-core/src/api/frontend.c:185

...

SUMMARY: AddressSanitizer: heap-buffer-overflow /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:117 in swap_copy_rom
Shadow bytes around the buggy address:
  0x6210002dfe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002dff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002dff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002e0000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002e0080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x6210002e0100:[03]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
...

Result for generated .n64 file:

$ ./BinAsan/Release/RMG /tmp/size-not-divisible-by-4.n64
=================================================================
==185584==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6210002e0100 at pc 0x7f88976f7dad bp 0x7f886d6190e0 sp 0x7f886d6190d0
READ of size 4 at 0x6210002e0100 thread T13 (Thread::Emulati)
    #0 0x7f88976f7dac in swap_copy_rom /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:130
    #1 0x7f88976f7fd9 in open_rom /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:159
    #2 0x7f8897631a9f in CoreDoCommand /tmp/RMG/Source/3rdParty/mupen64plus-core/src/api/frontend.c:185

...

SUMMARY: AddressSanitizer: heap-buffer-overflow /tmp/RMG/Source/3rdParty/mupen64plus-core/src/main/rom.c:130 in swap_copy_rom
Shadow bytes around the buggy address:
  0x6210002dfe80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002dff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002dff80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002e0000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x6210002e0080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x6210002e0100:[03]fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0200: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0280: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0300: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x6210002e0380: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
...

While it does not seem to be a serious issue, it's still better to avoid out-of-bounds memory access. This problem could be fixed easily by making sure that input ROM file size is always divisible by 4 (romsize % 4 == 0). Such check could be added either in swap_copy_rom, open_rom or CoreDoCommand -> M64CMD_ROM_OPEN. Even if there are cases where N64 ROM size could be not divisible by 4, then emulators could simply pass ROM buffer with size bigger than needed, to comply with size divisibility rule.

Test platform

  • Ubuntu 23.04 (x86-64)
  • RMG emulator (https://github.com/Rosalie241/RMG)
    • built from master branch
    • Release build, x86-64
    • custom flags added to mupen64plus-core Makefile:
      • CFLAGS += -O0 -g -fsanitize=address
      • -O0 and -g for better crash backtraces in Release build
      • -fsanitize=address to compile with AddressSanitizer
  • gdb 13.1 for debugging
richard42 commented

This is really great diagnostic work that you've done here. Can you file pull requests for fixes for these bugs? I would recommend addressing both of these problems in CoreDoCommand()