search

libgme / game-music-emu

Blargg's video game music emulation library, which allows audio applications to easily add playback support for the music of many classic video game consoles.
GNU Lesser General Public License v2.1
72 stars 14 forks source link

segfault caused by null pointer in Hes_Cpu::run #10

Closed Wohlstand closed 7 years ago

Wohlstand commented 7 years ago

Original report by Hanno Böck (Bitbucket: [Hanno Böck](https://bitbucket.org/Hanno Böck), ).

The attached file crashes game-music-emu with a null pointer access.

This was found with the fuzzing tool american fuzzy lop.

Here's a stack trace from address sanitizer:

==6156==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x0000005b1870 bp 0x7ffe17a8ed40 sp 0x7ffe17a8eae0 T0)
==6156==The signal is caused by a READ memory access.
==6156==Hint: address points to the zero page.
    #0 0x5b186f in Hes_Cpu::run(int) /mnt/ram/game-music-emu-0.6.1/gme/Hes_Cpu.cpp:171:12
    #1 0x5323f1 in Hes_Emu::run_clocks(int&, int) /mnt/ram/game-music-emu-0.6.1/gme/Hes_Emu.cpp:511:12
    #2 0x55b019 in Classic_Emu::play_(long, short*) /mnt/ram/game-music-emu-0.6.1/gme/Classic_Emu.cpp:113:4
    #3 0x51d638 in Music_Emu::emu_play(long, short*) /mnt/ram/game-music-emu-0.6.1/gme/Music_Emu.cpp:305:23
    #4 0x51d638 in Music_Emu::fill_buf() /mnt/ram/game-music-emu-0.6.1/gme/Music_Emu.cpp:327
    #5 0x51ce9e in Music_Emu::start_track(int) /mnt/ram/game-music-emu-0.6.1/gme/Music_Emu.cpp:150:4
    #6 0x514387 in main /mnt/ram/game-music-emu-0.6.1/demo/basics.c:26:16
    #7 0x7f0034ad678f in __libc_start_main (/lib64/libc.so.6+0x2078f)
    #8 0x426dc8 in _start (/r/gmu/demo+0x426dc8)
Wohlstand commented 7 years ago

Original comment by Michael Pyne (Bitbucket: mpyne, GitHub: mpyne).

This seems to be due to the pc register using a "fast" integer type instead of a fixed-width type. If pc could be forced to the right value then maybe it could be possible for pc itself to be treated as a negative number whose lower 16 bits happens to have the exact magnitude if treated as positive.

This would cause the instr += PAGE_OFFSET( pc ) line to cause instr to equal 0, which is then dereferenced --> BOOM.

Commit 9728bf777a048fd9181bbe173da8fed5eae0ab99 changed the fast integer types to use fixed-width types. I can't reproduce the crash here with the latest master so I think it's already fixed, though I would appreciate someone else testing to ensure it's not just a local fix.

Wohlstand commented 7 years ago

Original comment by Hanno Böck (Bitbucket: [Hanno Böck](https://bitbucket.org/Hanno Böck), ).

Can confirm that this is fixed. I'm now re-testing with the git master branch to see if I find further bugs.

Wohlstand commented 7 years ago

Original comment by Hanno Böck (Bitbucket: [Hanno Böck](https://bitbucket.org/Hanno Böck), ).

already fixed by previous change in git