Konstanty / libmodplug

libmodplug GitHub repository
Other
134 stars 41 forks source link

Segmentation fault in mmreadUBYTE(MMFILE*) #56

Open CounterPillow opened 3 years ago

CounterPillow commented 3 years ago
  1. build ffmpeg with libmodplug support enabled
  2. build mpv against that ffmpeg
  3. play this midi file: https://0x0.st/--fc.mid
  4. Get this backtrace:
    (gdb) bt full
    #0  0x00007ffff788744e in mmreadUBYTE(MMFILE*) (mmfile=0x7fffe66ee4a0)
    at /home/fratti/Projekte/libmodplug/src/load_mid.cpp:133
        b = 0 '\000'
    #1  0x00007ffff7888bab in mid_read_byte(MIDHANDLE*) (h=0x7fffb0001eb0)
    at /home/fratti/Projekte/libmodplug/src/load_mid.cpp:725
    #2  0x00007ffff788ae1f in CSoundFile::ReadMID(unsigned char const*, unsigned int)
    (this=0x7fffb000ada0, lpStream=0x7fffb00773c0 "MThd", dwMemLength=5242880)
    at /home/fratti/Projekte/libmodplug/src/load_mid.cpp:1420
        avoid_reentry = 1
        h = 0x7fffb0001eb0
        mm = {
          mm = 0x7fffb00773c0 "MThd",
          sz = 5242880,
          pos = 30022720
        }
        ch = 15
        dmulti = 1
        maxtempo = 126
        panlow = 0
        panhigh = 177
        numchans = -1341688896
        numtracks = 32767
        ttp = 0x0
        t = 13
        numpats = 0
        buf = "\000\000\000\000\b\001\000\000\000\000\000\000%\000\000\000\000\000\000\000\020\355α\377\177\000\000\022\305\342", '\000' <repeats 224 times>
        miditracklen = 2122740291
        runningstatus = 255 '\377'
        cmd = 240 '\360'
        midibyte = "\034"
        metalen = 1768455
        delta = 0
        p = 0x7fffe66ee4c0 ""
    #3  0x00007ffff78a7efe in CSoundFile::Create(unsigned char const*, unsigned int)
    (this=0x7fffb000ada0, lpStream=0x7fffb00773c0 "MThd", dwMemLength=5242880)
    at /home/fratti/Projekte/libmodplug/src/sndfile.cpp:147
        bMMCmp = false
        i = -141957232
        pins = 0x7fffb000ada0
    #4  0x00007ffff789e7f3 in ModPlug_Load(void const*, int) (data=0x7fffb00773c0, size=5242880)
    at /home/fratti/Projekte/libmodplug/src/modplug.cpp:88
        result = 0x7fffb000ada0
AliceLR commented 3 years ago

I've been fuzzing libmodplug to find bugs in #57 and, despite finding one bug in that, I've mostly found a lot of bugs elsewhere instead. Particularly, there is some pretty bad cleanup and bounds checking in the MIDI loader, which is almost certainly the cause of this crash. There is also at least one place where the loader can return without clearing the reentrancy flag, leading to indefinite hangs. Whenever I get a run that hits over 1m executions I'll get my collection of fixes ready to submit as a patch (should be soon).

Edit: it's here #58. I haven't confirmed that this patch fixed it on the OP's end, but I added the input file to the corpus and it hasn't crashed.

sezero commented 3 years ago

The midi files support (load_mid, load_pat, load_abc) in libmodplug is bad and should die. I specifically disable it in my fork, SDL_sound removed it in its fork and went back to timidity.

AliceLR commented 3 years ago

Many of the crash bugs and slow loads I've encountered fuzzing libmodplug so far are from the MIDI loader. This loader does not seem to have been written with stability or performance in mind.

IMO MIDI support makes sense from the perspective of importing the data into a tracker for creating a module, but not so much for playback (as there are much better players).

edit: this pretty much sums up the code quality of the MIDI loaders from what I've seen so far.

static BYTE pat_gm_used[MAXSMP];
int pat_numsmp()
{
        return strlen((const char *)pat_gm_used);
}

int pat_numinstr(void)
{
        return strlen((const char *)pat_gm_used);
}
AliceLR commented 2 years ago

This should be fixed via #66 / #67 and maybe #76.