heap-buffer-overflow in WildMidi_Open

[gy741]

Hi.

I found a heap-buffer-overflow in wildmidi.

Please confirm.

Thanks.

Summary: heap-buffer-overflow
Browser/OS: Ubuntu 16.04 64bit
Steps to reproduce:
1.Download the .POC files.
2.Compile the source code with ASan or Run wildmidi as valgrind.
3.Execute the following command
: ./wildmidi $PoC -o /dev/null
PoC download : PoC

valgrind log
------------
==15888== Conditional jump or move depends on uninitialised value(s)
==15888==    at 0x4E3E032: WildMidi_Open (wildmidi_lib.c:1659)
==15888==
===========================================
==18430==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000000037 at pc 0x00000049e9fd bp 0x7ffd1ac09070 sp 0x7ffd1ac08820
READ of size 8 at 0x602000000037 thread T0
 #0 0x49e9fc in __interceptor_memcmp.part.75 (/root/karas/wildmidi/wildmidi+0x49e9fc)
 #1 0x7f54ee23f150 in WildMidi_Open /root/karas/wildmidi/src/wildmidi_lib.c:1658:9
 #2 0x50db5c in main /root/karas/wildmidi/src/wildmidi.c:1804:24
 #3 0x7f54ed32b82f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
 #4 0x41a698 in _start (/root/karas/wildmidi/wildmidi+0x41a698)
==18430==ABORTING

[sezero]

memcmp() doesn't know how long the mididata buffer is, therefore comparing
a 6 bytes buffer to 8 bytes "HMIMIDIP" or 18 bytes "HMI-MIDISONG061595" is
reading beyond the limits.

Requiring at least 18 bytes of midisize before the memcmp() checks should
fix this, like below. Should I apply?

    if (midisize < 18) {
        _WM_GLOBAL_ERROR(__FUNCTION__, __LINE__, WM_ERR_CORUPT, "(too short)", 0);
        return (NULL);
    }

[psi29a]

Please do.
'HMI-MIDISONG061595' is the biggest header we have yet to compare with, perhaps we should put additional comment in for why we chose 18 as a magic number so that we can up it's value if we ever have to compare against another longer string. Does that make sense?

[sezero]

Done. Fixed with commit 814f31d

[sezero]

... comment in for why we chose 18 as a magic number

Only two places with 18: our size check and the memcmp for 'HMI-MIDISONG061595'
so it should be obvious. No need, IMO.

[psi29a]

OK