Out-of-bounds read in pat_smplooped() — missing lower bound, reachable from a 32-byte MIDI file

[perparimmjeku]

Summary

pat_smplooped() checks only the upper bound of its index and then subtracts one,
so an index of 0 reads one byte before the pat_loops global.

src/load_pat.cpp:215:

static BYTE pat_loops[MAXSMP];                       /* :146, MAXSMP == 191 */

int pat_smplooped(int smp)
{
	if( smp < MAXSMP ) return pat_loops[smp - 1];    /* no lower bound */
	return 1;
}

The index arrives from parsed MIDI event data:

ModPlug_Load                         modplug.cpp:88
  CSoundFile::Create                 sndfile.cpp:146
    CSoundFile::ReadMID              load_mid.cpp:1548
      MID_ReadPatterns               load_mid.cpp:905
        mid_next_note                load_mid.cpp:835
          mid_is_note_event          load_mid.cpp:829
            pat_smplooped(e->smpno)  <-- e->smpno not constrained to >= 1

PoC — 32 bytes

00000000: 4d54 6864 0000 0006 ffff 0001 0060 4d54  MThd.........`MT
00000010: 726b 0000 0008 0090 4040 1a00 b000 b007  rk......@@......

ERROR: AddressSanitizer: global-buffer-overflow
READ of size 1 at 0x000102d582bf
    #0 in pat_smplooped                 load_pat.cpp:215
    #1 in mid_is_note_event(_MIDEVENT*)  load_mid.cpp:829
    #6 in ModPlug_Load                   modplug.cpp:88

0x000102d582bf is located 1 bytes before global variable 'pat_loops'
defined in 'src/load_pat.cpp' (0x000102d582c0) of size 191

Built at d1b97ed0020bc620a059d3675d1854b40bd2608d with
-fsanitize=address (cmake -DCMAKE_POLICY_VERSION_MINIMUM=3.5).

Fix

 int pat_smplooped(int smp)
 {
-	if( smp < MAXSMP ) return pat_loops[smp - 1];
+	if( smp > 0 && smp < MAXSMP ) return pat_loops[smp - 1];
 	return 1;
 }

Verified: the PoC runs clean against the patched build.

Also worth a look

The matching write at load_pat.cpp:1092, pat_loops[smp-1] = ..., has no
visible lower-bound check either. In the paths I exercised smp arrives >= 1, so
I did not reach it and am not reporting it as a bug — but it is the same idiom.

Impact

libmodplug is vendored widely (media players, game engines, distro packages) and
routinely parses untrusted files, so this is a crash reachable from a tiny input
in any hardened build. The out-of-bounds byte also decides whether a note event
loops, so it influences playback state.