Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segmentation fault in mmreadUBYTE(MMFILE*) #56

Open
CounterPillow opened this issue Jan 26, 2021 · 4 comments
Open

Segmentation fault in mmreadUBYTE(MMFILE*) #56

CounterPillow opened this issue Jan 26, 2021 · 4 comments

Comments

@CounterPillow
Copy link

  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
Copy link
Contributor

AliceLR commented Jun 13, 2021

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
Copy link
Contributor

sezero commented Jun 13, 2021

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
Copy link
Contributor

AliceLR commented Jun 13, 2021

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
Copy link
Contributor

AliceLR commented Jan 29, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants