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
68 stars 12 forks source link

gameboy emulation causes invalid memory read #9

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), ).


When compiling game-music-emu with address sanitizer and trying to play a gameboy music file (I tested with this one [1]) I see an invalid memory read in version 0.6.1.

The reason is this code:

#!c

    static unsigned char const initial_wave [] = {
        0x84,0x40,0x43,0xAA,0x2D,0x78,0x92,0x3C, // wave table
        0x60,0x59,0x59,0xB0,0x34,0xB8,0x2E,0xDA
    };
    memcpy( wave.wave, initial_wave, sizeof wave.wave );

It seems "sizeof wave.wave" is 32, however the initial_wave array is only 16 bytes. Therefore this overreads. I think just changing "sizeof wave.wave" to "16" should fix this (at least I can play the file after that), but this should probably be reviewed by someone familiar with the code.

[1] http://www.zophar.net/music/gameboy-gbs/102-dalmatians-puppies-to-the-rescue

Here's the full error message from address sanitizer:

==7807==ERROR: AddressSanitizer: global-buffer-overflow on address 0x000000632c90 at pc 0x0000004c0bb5 bp 0x7fffea986480 sp 0x7fffea985c30
READ of size 32 at 0x000000632c90 thread T0
    #0 0x4c0bb4 in __asan_memcpy (/mnt/ram/game-music-emu-0.6.1/b/player/gme_player+0x4c0bb4)
    #1 0x5a7a1d in Gb_Apu::reset() /mnt/ram/game-music-emu-0.6.1/gme/Gb_Apu.cpp:126:2
    #2 0x5a7a1d in Gb_Apu::Gb_Apu() /mnt/ram/game-music-emu-0.6.1/gme/Gb_Apu.cpp:48
    #3 0x52d58d in Gbs_Emu::Gbs_Emu() /mnt/ram/game-music-emu-0.6.1/gme/Gbs_Emu.cpp:26:10
    #4 0x52fb8f in new_gbs_emu() /mnt/ram/game-music-emu-0.6.1/gme/Gbs_Emu.cpp:100:54
    #5 0x519477 in gme_new_emu /mnt/ram/game-music-emu-0.6.1/gme/gme.cpp:197:19
    #6 0x519e2e in gme_open_file /mnt/ram/game-music-emu-0.6.1/gme/gme.cpp:174:19
    #7 0x514f1b in Music_Player::load_file(char const*) /mnt/ram/game-music-emu-0.6.1/player/Music_Player.cpp:78:2
    #8 0x5164be in main /mnt/ram/game-music-emu-0.6.1/player/player.cpp:88:24
    #9 0x7f3e27a39690 in __libc_start_main /var/tmp/portage/sys-libs/glibc-2.23-r3/work/glibc-2.23/csu/../csu/libc-start.c:289
    #10 0x427898 in _start (/mnt/ram/game-music-emu-0.6.1/b/player/gme_player+0x427898)

0x000000632c90 is located 48 bytes to the left of global variable '<string literal>' defined in '/mnt/ram/game-music-emu-0.6.1/gme/Gb_Apu.cpp:125:12' (0x632cc0) of size 5
  '<string literal>' is ascii string '%i 
'
0x000000632c90 is located 0 bytes to the right of global variable 'initial_wave' defined in '/mnt/ram/game-music-emu-0.6.1/gme/Gb_Apu.cpp:122:29' (0x632c80) of size 16
Wohlstand commented 7 years ago

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


proposed patch

Wohlstand commented 7 years ago

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


Duplicate of #5.

Wohlstand commented 7 years ago

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


As noted this was already reported and fixed in issue #5, but it's good to see that ASan is able to detect this class of bug!