search

mupen64plus / mupen64plus-core

Core module of the Mupen64Plus project
1.32k stars 258 forks source link

Missing ROM file size validation #1049

Closed 269261 closed 10 months ago

269261 commented 1 year ago

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:

https://github.com/mupen64plus/mupen64plus-core/blob/f500eb58f76e636e231c3cc2b3d904210f0677c9/src/main/rom.c#L159

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. https://github.com/mupen64plus/mupen64plus-core/issues/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:

https://github.com/mupen64plus/mupen64plus-core/blob/f500eb58f76e636e231c3cc2b3d904210f0677c9/src/main/rom.c#L107-L119 https://github.com/mupen64plus/mupen64plus-core/blob/f500eb58f76e636e231c3cc2b3d904210f0677c9/src/main/rom.c#L120-L132

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:

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

richard42 commented 10 months ago

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()