search

LIJI32 / SameBoy

Game Boy and Game Boy Color emulator written in C
https://sameboy.github.io/
Other
1.58k stars 205 forks source link

Fix oob reads in debugger command parsing #560

Closed thestr4ng3r closed 10 months ago

thestr4ng3r commented 10 months ago

The length of only one of the operand strings was checked before the memcmp in these cases, causing out-of-bounds reads when the other was shorter. This could be seen by compiling with ASAN and for example executing any command longer than 2 characters.

In particular, this fixes the following issues:

SameBoy v0.15.8
> asdf
> 
=================================================================
==84973==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000102fc35e3 at pc 0x00010379b520 bp 0x00016cf95e90 sp 0x00016cf95628
READ of size 4 at 0x000102fc35e3 thread T0
    #0 0x10379b51c in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x11c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b51c) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x10379b970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x102e95440 in find_command debugger.c:2190
    #3 0x102e94f0c in GB_debugger_execute_command debugger.c:2384
    #4 0x102e97cd0 in GB_debugger_handle_async_commands debugger.c:2598
    #5 0x102efb538 in GB_run gb.c:1191
    #6 0x102fa9488 in run main.c:859
    #7 0x102fa6c7c in main main.c:1151
    #8 0x189deff24  (<unknown module>)

0x000102fc35e3 is located 61 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2139:23' (0x102fc3620) of size 24
  '<string literal>' is ascii string 'Revert the last command'
0x000102fc35e3 is located 29 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2139:6' (0x102fc3600) of size 5
  '<string literal>' is ascii string 'undo'
0x000102fc35e3 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:2137:6' (0x102fc35e0) of size 3
  '<string literal>' is ascii string 'bs'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> reset aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<tab>
=================================================================
==85648==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000102aac5c6 at pc 0x00010328360c bp 0x00016d7f2930 sp 0x00016d7f20c8
READ of size 36 at 0x000102aac5c6 thread T5
    #0 0x103283608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x103283970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x10298fdec in reset_completer debugger.c:786
    #3 0x10297da20 in GB_debugger_complete_substring debugger.c:2458
    #4 0x102a8f510 in completer main.c:55
    #5 0x102a74464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000102aac5c6 is located 58 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:828:20' (0x102aac600) of size 67
  '<string literal>' is ascii string 'ROM reloading via the debugger is not supported in this frontend.
'
0x000102aac5c6 is located 26 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:816:35' (0x102aac5e0) of size 7
  '<string literal>' is ascii string 'reload'
0x000102aac5c6 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:808:35' (0x102aac5c0) of size 6
  '<string literal>' is ascii string 'quick'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> softbreak aaaaa<tab>
=================================================================
==85929==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000100968fc3 at pc 0x00010113f60c bp 0x00016f81e930 sp 0x00016f81e0c8
READ of size 10 at 0x000100968fc3 thread T4
    #0 0x10113f608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x10113f970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x1008578e0 in on_off_completer debugger.c:909
    #3 0x100839a20 in GB_debugger_complete_substring debugger.c:2458
    #4 0x10094b510 in completer main.c:55
    #5 0x100930464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000100968fc3 is located 61 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:1330:20' (0x100969000) of size 21
  '<string literal>' is ascii string 'No breakpoints set.
'
0x000100968fc3 is located 29 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:924:40' (0x100968fe0) of size 4
  '<string literal>' is ascii string 'off'
0x000100968fc3 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:921:35' (0x100968fc0) of size 3
  '<string literal>' is ascii string 'on'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> aaaa<tab>
=================================================================
==86259==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0001006835e3 at pc 0x000100e5b520 bp 0x00016fb02ab0 sp 0x00016fb02248
READ of size 4 at 0x0001006835e3 thread T4
    #0 0x100e5b51c in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x11c (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b51c) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x100e5b970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x1005558b4 in GB_debugger_complete_substring debugger.c:2448
    #3 0x100667510 in completer main.c:55
    #4 0x10064c464 in mainloop console.c:813
    #5 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #6 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x0001006835e3 is located 61 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2139:23' (0x100683620) of size 24
  '<string literal>' is ascii string 'Revert the last command'
0x0001006835e3 is located 29 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2139:6' (0x100683600) of size 5
  '<string literal>' is ascii string 'undo'
0x0001006835e3 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:2137:6' (0x1006835e0) of size 3
  '<string literal>' is ascii string 'bs'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> print aaaaaaaaaaaaaaaaaaaaaaaaaaaaa<tab>
=================================================================
==86578==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x000105e356b6 at pc 0x00010348b60c bp 0x00016d4d2910 sp 0x00016d4d20a8
READ of size 29 at 0x000105e356b6 thread T4
    #0 0x10348b608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x10348b970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x102b9b814 in symbol_completer debugger.c:974
    #3 0x102b85a20 in GB_debugger_complete_substring debugger.c:2458
    #4 0x102c97510 in completer main.c:55
    #5 0x102c7c464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000105e356b6 is located 0 bytes to the right of 6-byte region [0x000105e356b0,0x000105e356b6)
allocated by thread T0 here:
    #0 0x1034addc8 in wrap_strdup+0x1c4 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3ddc8) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x102c6d618 in GB_map_add_symbol symbol_hash.c:35
    #2 0x102b883e8 in GB_debugger_add_symbol debugger.c:2616
    #3 0x102b88b84 in GB_debugger_load_symbol_file debugger.c:2644
    #4 0x102c99324 in run main.c:834
    #5 0x102c96c64 in main main.c:1151
    #6 0x189deff24  (<unknown module>)

Thread T4 created by T0 here:
    #0 0x1034acd2c in wrap_pthread_create+0x54 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x3cd2c) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x102c7ac44 in CON_start console.c:891
    #2 0x102c959d0 in main main.c:1030
    #3 0x189deff24  (<unknown module>)

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> breakpoint/jjjjjjjjjjjjj<tab>
=================================================================
==86917==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000104e87b82 at pc 0x00010565f60c bp 0x00016b2fe930 sp 0x00016b2fe0c8
READ of size 13 at 0x000104e87b82 thread T4
    #0 0x10565f608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x10565f970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x104d72dbc in j_completer debugger.c:986
    #3 0x104d59af4 in GB_debugger_complete_substring debugger.c:2465
    #4 0x104e6b510 in completer main.c:55
    #5 0x104e50464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000104e87b82 is located 62 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2157:27' (0x104e87bc0) of size 58
  '<string literal>' is ascii string 'Delete a breakpoint by its identifier, or all breakpoints'
0x000104e87b82 is located 30 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2157:6' (0x104e87ba0) of size 7
  '<string literal>' is ascii string 'delete'
0x000104e87b82 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:2155:113' (0x104e87b80) of size 2
  '<string literal>' is ascii string 'j'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> watch/aaaaaaaaaaaaaaaaaaaaaaaa<tab>
=================================================================
==87382==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000104d56ac2 at pc 0x00010552f60c bp 0x00016b42e910 sp 0x00016b42e0a8
READ of size 24 at 0x000104d56ac2 thread T4
    #0 0x10552f608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x10552f970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x104c46220 in rw_completer debugger.c:1163
    #3 0x104c29af4 in GB_debugger_complete_substring debugger.c:2465
    #4 0x104d3b510 in completer main.c:55
    #5 0x104d20464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000104d56ac2 is located 62 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2715:5' (0x104d56b00) of size 21
  '<string literal>' is ascii string 'GB_debugger_evaluate'
0x000104d56ac2 is located 30 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:2643:26' (0x104d56ae0) of size 9
  '<string literal>' is ascii string '%x:%x %s'
0x000104d56ac2 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:2624:27' (0x104d56ac0) of size 2
  '<string literal>' is ascii string 'r'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> print/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<tab>
=================================================================
==87688==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000100b0c9a2 at pc 0x0001012e360c bp 0x00016f67a8d0 sp 0x00016f67a068
READ of size 38 at 0x000100b0c9a2 thread T4
    #0 0x1012e3608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x1012e3970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x1009f3c40 in format_completer debugger.c:1433
    #3 0x1009ddaf4 in GB_debugger_complete_substring debugger.c:2465
    #4 0x100aef510 in completer main.c:55
    #5 0x100ad4464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000100b0c9a2 is located 62 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:1464:28' (0x100b0c9e0) of size 5
  '<string literal>' is ascii string '=%d
'
0x000100b0c9a2 is located 30 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:1461:28' (0x100b0c9c0) of size 5
  '<string literal>' is ascii string '=%s
'
0x000100b0c9a2 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:1449:21' (0x100b0c9a0) of size 2
  '<string literal>' is ascii string 'a'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> ticks aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<tab>
=================================================================
==88003==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000102b3d2a5 at pc 0x00010331360c bp 0x00016d64a930 sp 0x00016d64a0c8
READ of size 38 at 0x000102b3d2a5 thread T4
    #0 0x103313608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x103313970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x102a2e8f4 in keep_completer debugger.c:1681
    #3 0x102a0da20 in GB_debugger_complete_substring debugger.c:2458
    #4 0x102b1f510 in completer main.c:55
    #5 0x102b04464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000102b3d2a5 is located 59 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:1703:16' (0x102b3d2e0) of size 16
  '<string literal>' is ascii string 'M-cycles: %llu
'
0x000102b3d2a5 is located 27 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:1702:16' (0x102b3d2c0) of size 16
  '<string literal>' is ascii string 'T-cycles: %llu
'
0x000102b3d2a5 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:1694:35' (0x102b3d2a0) of size 5
  '<string literal>' is ascii string 'keep'

SameBoy v0.15.8
Wrote 00 to ff7f (HW Register)
> wave/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa<tab>
=================================================================
==88307==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000104eee342 at pc 0x0001056c360c bp 0x00016b29a910 sp 0x00016b29a0a8
READ of size 31 at 0x000104eee342 thread T4
    #0 0x1056c3608 in MemcmpInterceptorCommon(void*, int (*)(void const*, void const*, unsigned long), void const*, void const*, unsigned long)+0x208 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b608) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #1 0x1056c3970 in wrap_memcmp+0xac (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1b970) (BuildId: f0a7ac5c49bc3abc851181b6f92b308a32000000200000000100000000000b00)
    #2 0x104de27cc in wave_completer debugger.c:1997
    #3 0x104dbdaf4 in GB_debugger_complete_substring debugger.c:2465
    #4 0x104ecf510 in completer main.c:55
    #5 0x104eb4464 in mainloop console.c:813
    #6 0x18a147fa4 in _pthread_start+0x90 (libsystem_pthread.dylib:arm64e+0x6fa4) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)
    #7 0x18a142d9c in thread_start+0x4 (libsystem_pthread.dylib:arm64e+0x1d9c) (BuildId: 1f30fb9abdf932dba7098417666a7e4532000000200000000100000000050d00)

0x000104eee342 is located 62 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:1995:44' (0x104eee380) of size 2
  '<string literal>' is ascii string 'l'
0x000104eee342 is located 30 bytes to the left of global variable '<string literal>' defined in 'Core/debugger.c:1995:39' (0x104eee360) of size 2
  '<string literal>' is ascii string 'f'
0x000104eee342 is located 0 bytes to the right of global variable '<string literal>' defined in 'Core/debugger.c:1995:34' (0x104eee340) of size 2
  '<string literal>' is ascii string 'c'
LIJI32 commented 10 months ago

I'm pretty sure this is an ASAN false positive? memcmp will early exit once it sees a non-matching null terminator so it shouldn't OOB read, but I guess "non-naïve" memcmp implementation which read register-sized data each iteration could crash under certain implementations and page-alignment scenarios. In any case, this fix seems to not break anything and might still prevent some implementation-specific crash, so I'll merge it in.

thestr4ng3r commented 10 months ago

No, memcmp never stops at null terminators:

The memcmp() function compares byte string s1 against byte string s2. Both strings are assumed to be n bytes long.

LIJI32 commented 10 months ago

It would stop at the null terminator because it is guaranteed to mismatch on strings of different lengths.

thestr4ng3r commented 10 months ago

Ok, I see what you mean now. Yeah that would depend on the implementation indeed.