segfault caused by null pointer in Hes_Cpu::run

Issue #8 resolved
Hanno Böck created an issue

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)

Comments (4)

  1. Michael Pyne repo owner

    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.

  2. Hanno Böck reporter

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

  3. Log in to comment