Skip to content

Commit 7ae694d

Browse files
AliceLRAliceLR
committed
Fix misc. bugs found by libfuzzer.
* Fix out-of-bounds reads in the DSMI AMF loader caused by missing order list bounds checks. * Fix out-of-bounds reads in the FAR loader caused by not correctly bounding the maximum pattern read size. * Fix out-of-bounds reads in the IT sample decompressors caused by allowing ITReadBits to read past the end of the sample buffer. * Fix out-of-bounds reads in the MED loader caused by the MMD2PLAYSEQ table bounds check not including the size of the offset being read. * Fix out-of-bounds reads in the MIDI loader caused by no bounds checks being performed on the mmread* and mid_read* functions. * Fix leaks in the MIDI loader caused by not freeing MIDTRACKs. * Fix leak and hangs in the MIDI loader caused by not releasing the MIDI structs and reentry flag when m_nChannels is 0. * Fix out-of-bounds reads in the OKT loader caused by numerous bounds checks not correctly including the size of the data being read. * Fix out-of-bounds reads in the PSM loader caused by dereferencing (PSMCHUNK *)lpStream before any bounds checks. * Fix out-of-bounds reads in the S3M loader due to various absent bounds checks. * Fix out-of-bounds reads in the ULT loader due to incorrect event bounding. * Fix out-of-bounds reads in the XM loader due to not including the size of XMSAMPLEHEADER in the XMSAMPLEHEADER bounds check. * Fix potential slowdown in ABC loader caused by sign extending in mmfgets instead of iterating on an unsigned int.
1 parent d7ba5ef commit 7ae694d

11 files changed

Lines changed: 91 additions & 50 deletions

‎src/load_abc.cpp‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -479,8 +479,9 @@ static int mmfgetc(MMFILE *mmfile)
479479

480480
static void mmfgets(char buf[], unsigned int bufsz, MMFILE *mmfile)
481481
{
482-
int i,b;
483-
for( i=0; i<(int)bufsz-1; i++ ) {
482+
unsigned int i;
483+
int b;
484+
for( i=0; bufsz && i<bufsz-1; i++ ) {
484485
b = mmfgetc(mmfile);
485486
if( b==EOF ) break;
486487
buf[i] = b;

‎src/load_amf.cpp‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,8 +317,12 @@ BOOL CSoundFile::ReadAMF(LPCBYTE lpStream, const DWORD dwMemLength)
317317
PatternSize[iOrd] = 64;
318318
if (pfh->version >= 14)
319319
{
320+
if (dwMemPos + m_nChannels * sizeof(USHORT) + 2 > dwMemLength) return FALSE;
320321
PatternSize[iOrd] = bswapLE16(*(USHORT *)(lpStream+dwMemPos));
321322
dwMemPos += 2;
323+
} else
324+
{
325+
if (dwMemPos + m_nChannels * sizeof(USHORT) > dwMemLength) return FALSE;
322326
}
323327
ptracks[iOrd] = (USHORT *)(lpStream+dwMemPos);
324328
dwMemPos += m_nChannels * sizeof(USHORT);

‎src/load_far.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ BOOL CSoundFile::ReadFAR(const BYTE *lpStream, DWORD dwMemLength)
139139
UINT patbrk = lpStream[dwMemPos];
140140
const BYTE *p = lpStream + dwMemPos + 2;
141141
UINT max = rows*16*4;
142-
if (max > patlen-2) max = patlen-2;
142+
if (max > ((patlen-2) & ~3)) max = ((patlen-2) & ~3);
143143
for (UINT len=0; len<max; len += 4, m++)
144144
{
145145
BYTE note = p[len];

‎src/load_it.cpp‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,7 +1182,7 @@ BOOL CSoundFile::SaveIT(LPCSTR lpszFileName, UINT nPacking)
11821182
//////////////////////////////////////////////////////////////////////////////
11831183
// IT 2.14 compression
11841184

1185-
DWORD ITReadBits(DWORD &bitbuf, UINT &bitnum, LPBYTE &ibuf, CHAR n)
1185+
DWORD ITReadBits(DWORD &bitbuf, UINT &bitnum, LPBYTE &ibuf, LPBYTE ibufend, CHAR n)
11861186
//-----------------------------------------------------------------
11871187
{
11881188
DWORD retval = 0;
@@ -1198,6 +1198,9 @@ DWORD ITReadBits(DWORD &bitbuf, UINT &bitnum, LPBYTE &ibuf, CHAR n)
11981198
{
11991199
if (!bitnum)
12001200
{
1201+
if (ibuf >= ibufend)
1202+
return 0;
1203+
12011204
bitbuf = *ibuf++;
12021205
bitnum = 8;
12031206
}
@@ -1218,6 +1221,7 @@ void ITUnpack8Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dwM
12181221
{
12191222
signed char *pDst = pSample;
12201223
LPBYTE pSrc = lpMemFile;
1224+
LPBYTE pStop = lpMemFile + dwMemLength;
12211225
DWORD wHdr = 0;
12221226
DWORD wCount = 0;
12231227
DWORD bitbuf = 0;
@@ -1241,13 +1245,13 @@ void ITUnpack8Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dwM
12411245
DWORD dwPos = 0;
12421246
do
12431247
{
1244-
WORD wBits = (WORD)ITReadBits(bitbuf, bitnum, pSrc, bLeft);
1248+
WORD wBits = (WORD)ITReadBits(bitbuf, bitnum, pSrc, pStop, bLeft);
12451249
if (bLeft < 7)
12461250
{
12471251
DWORD i = 1 << (bLeft-1);
12481252
DWORD j = wBits & 0xFFFF;
12491253
if (i != j) goto UnpackByte;
1250-
wBits = (WORD)(ITReadBits(bitbuf, bitnum, pSrc, 3) + 1) & 0xFF;
1254+
wBits = (WORD)(ITReadBits(bitbuf, bitnum, pSrc, pStop, 3) + 1) & 0xFF;
12511255
bLeft = ((BYTE)wBits < bLeft) ? (BYTE)wBits : (BYTE)((wBits+1) & 0xFF);
12521256
goto Next;
12531257
}
@@ -1285,7 +1289,7 @@ void ITUnpack8Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dwM
12851289
SkipByte:
12861290
dwPos++;
12871291
Next:
1288-
if (pSrc >= lpMemFile+dwMemLength+1) return;
1292+
if (pSrc >= pStop + 1) return;
12891293
} while (dwPos < d);
12901294
// Move On
12911295
wCount -= d;
@@ -1300,6 +1304,7 @@ void ITUnpack16Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dw
13001304
{
13011305
signed short *pDst = (signed short *)pSample;
13021306
LPBYTE pSrc = lpMemFile;
1307+
LPBYTE pStop = lpMemFile + dwMemLength;
13031308
DWORD wHdr = 0;
13041309
DWORD wCount = 0;
13051310
DWORD bitbuf = 0;
@@ -1324,13 +1329,13 @@ void ITUnpack16Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dw
13241329
DWORD dwPos = 0;
13251330
do
13261331
{
1327-
DWORD dwBits = ITReadBits(bitbuf, bitnum, pSrc, bLeft);
1332+
DWORD dwBits = ITReadBits(bitbuf, bitnum, pSrc, pStop, bLeft);
13281333
if (bLeft < 7)
13291334
{
13301335
DWORD i = 1 << (bLeft-1);
13311336
DWORD j = dwBits;
13321337
if (i != j) goto UnpackByte;
1333-
dwBits = ITReadBits(bitbuf, bitnum, pSrc, 4) + 1;
1338+
dwBits = ITReadBits(bitbuf, bitnum, pSrc, pStop, 4) + 1;
13341339
bLeft = ((BYTE)(dwBits & 0xFF) < bLeft) ? (BYTE)(dwBits & 0xFF) : (BYTE)((dwBits+1) & 0xFF);
13351340
goto Next;
13361341
}
@@ -1368,13 +1373,13 @@ void ITUnpack16Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dw
13681373
SkipByte:
13691374
dwPos++;
13701375
Next:
1371-
if (pSrc >= lpMemFile+dwMemLength+1) return;
1376+
if (pSrc >= pStop + 1) return;
13721377
} while (dwPos < d);
13731378
// Move On
13741379
wCount -= d;
13751380
dwLen -= d;
13761381
pDst += d;
1377-
if (pSrc >= lpMemFile+dwMemLength) break;
1382+
if (pSrc >= pStop) break;
13781383
}
13791384
}
13801385

‎src/load_med.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -662,7 +662,7 @@ BOOL CSoundFile::ReadMed(const BYTE *lpStream, DWORD dwMemLength)
662662
}
663663
UINT pseq = 0;
664664

665-
if ((playseqtable) && (playseqtable < dwMemLength) && (nplayseq*4 < dwMemLength - playseqtable))
665+
if ((playseqtable) && (playseqtable < dwMemLength) && (nplayseq*4 + 4 < dwMemLength - playseqtable))
666666
{
667667
pseq = bswapBE32(((LPDWORD)(lpStream+playseqtable))[nplayseq]);
668668
}

‎src/load_mid.cpp‎

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ typedef struct {
105105
char *mm;
106106
unsigned int sz;
107107
int pos;
108+
int err;
108109
} MMFILE;
109110

110111
static void mmfseek(MMFILE *mmfile, long p, int whence)
@@ -130,21 +131,38 @@ static long mmftell(MMFILE *mmfile)
130131
static BYTE mmreadUBYTE(MMFILE *mmfile)
131132
{
132133
BYTE b;
134+
if ((unsigned int)mmfile->pos >= mmfile->sz)
135+
{
136+
mmfile->err = EOF;
137+
return 0;
138+
}
133139
b = (BYTE)mmfile->mm[mmfile->pos];
134140
mmfile->pos++;
135141
return b;
136142
}
137143

138-
static void mmreadUBYTES(BYTE *buf, long sz, MMFILE *mmfile)
144+
static unsigned long mmreadUBYTES(BYTE *buf, unsigned long sz, MMFILE *mmfile)
139145
{
146+
if ((unsigned long)mmfile->pos + sz >= mmfile->sz)
147+
{
148+
mmfile->err = EOF;
149+
return 0;
150+
}
140151
memcpy(buf, &mmfile->mm[mmfile->pos], sz);
141152
mmfile->pos += sz;
153+
return sz;
142154
}
143155

144-
static void mmreadSBYTES(char *buf, long sz, MMFILE *mmfile)
156+
static unsigned long mmreadSBYTES(char *buf, long sz, MMFILE *mmfile)
145157
{
158+
if ((unsigned long)mmfile->pos + sz >= mmfile->sz)
159+
{
160+
mmfile->err = EOF;
161+
return 0;
162+
}
146163
memcpy(buf, &mmfile->mm[mmfile->pos], sz);
147164
mmfile->pos += sz;
165+
return sz;
148166
}
149167

150168
/**********************************************************************/
@@ -709,14 +727,15 @@ static void mid_add_pitchwheel(MIDHANDLE *h, int mch, int wheel)
709727
static uint32_t mid_read_long(MIDHANDLE *h)
710728
{
711729
BYTE buf[4];
712-
mmreadUBYTES(buf, 4, h->mmf);
730+
if (!mmreadUBYTES(buf, 4, h->mmf)) return -1;
731+
713732
return (buf[0]<<24)|(buf[1]<<16)|(buf[2]<<8)|buf[3];
714733
}
715734

716735
static short int mid_read_short(MIDHANDLE *h)
717736
{
718737
BYTE buf[2];
719-
mmreadUBYTES(buf, 2, h->mmf);
738+
if (!mmreadUBYTES(buf, 2, h->mmf)) return -1;
720739
return (buf[0]<<8)|buf[1];
721740
}
722741

@@ -750,6 +769,7 @@ BOOL CSoundFile::TestMID(const BYTE *lpStream, DWORD dwMemLength)
750769
MMFILE mm;
751770
mm.mm = (char *)lpStream;
752771
mm.sz = dwMemLength;
772+
mm.err = 0;
753773
h.mmf = &mm;
754774
if (h.mmf->sz < 4) return FALSE;
755775
mmfseek(h.mmf,0,SEEK_SET);
@@ -791,6 +811,7 @@ static void MID_CleanupTracks(MIDHANDLE *handle)
791811
for( tp=handle->track; tp; tp = tn ) {
792812
tn=tp->next;
793813
MID_CleanupTrack(tp);
814+
free(tp);
794815
}
795816
handle->track = NULL;
796817
}
@@ -1172,19 +1193,14 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
11721193
BYTE *p;
11731194
while( avoid_reentry ) sleep(1);
11741195
avoid_reentry = 1;
1175-
if( !TestMID(lpStream, dwMemLength) ) {
1176-
avoid_reentry = 0;
1177-
return FALSE;
1178-
}
1196+
if( !TestMID(lpStream, dwMemLength) ) goto ErrorExit;
11791197
h = MID_Init();
1180-
if( !h ) {
1181-
avoid_reentry = 0;
1182-
return FALSE;
1183-
}
1198+
if( !h ) goto ErrorExit;
11841199
h->mmf = &mm;
11851200
mm.mm = (char *)lpStream;
11861201
mm.sz = dwMemLength;
11871202
mm.pos = 0;
1203+
mm.err = 0;
11881204
h->debug = getenv(ENV_MMMID_DEBUG);
11891205
h->verbose = getenv(ENV_MMMID_VERBOSE);
11901206
pat_resetsmp();
@@ -1193,6 +1209,8 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
11931209
h->midiformat = mid_read_short(h);
11941210
h->miditracks = mid_read_short(h);
11951211
h->resolution = mid_read_short(h);
1212+
if (mm.err) goto ErrorCleanup;
1213+
11961214
// at this point the h->mmf is positioned at first miditrack
11971215
if( h->midiformat == 0 ) h->miditracks = 1;
11981216
if( h->resolution & 0x8000 )
@@ -1205,11 +1223,8 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
12051223
m_nDefaultTempo = 0;
12061224
h->tracktime = 0;
12071225
h->speed = 6;
1208-
if (h->miditracks == 0) {
1209-
MID_Cleanup(h);
1210-
avoid_reentry = 0;
1211-
return FALSE;
1212-
}
1226+
if (h->miditracks == 0) goto ErrorCleanup;
1227+
12131228
p = (BYTE *)getenv(ENV_MMMID_SPEED);
12141229
if( p && isdigit(*p) && p[0] != '0' && p[1] == '\0' ) {
12151230
// transform speed
@@ -1247,19 +1262,18 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
12471262
}
12481263
for( t=0; t<(uint32_t)h->miditracks; t++ ) {
12491264
if( h->verbose ) printf("Parsing track %d\n", t+1);
1250-
mmreadSBYTES(buf,4,h->mmf);
1265+
if (!mmreadSBYTES(buf,4,h->mmf)) goto ErrorCleanup;
12511266
buf[4] = '\0';
12521267
if( strcmp(buf,"MTrk") ) {
12531268
mid_message("invalid track-chunk '%s' is not 'MTrk'",buf);
1254-
MID_Cleanup(h);
1255-
avoid_reentry = 0;
1256-
return FALSE;
1269+
goto ErrorCleanup;
12571270
}
12581271
miditracklen = mid_read_long(h);
1259-
if (mm.sz < miditracklen) continue;
1272+
if (mm.err || mm.sz < miditracklen) continue;
12601273
runningstatus = 0;
12611274
if( t && h->midiformat == 1 ) mid_rewind_tracks(h); // tracks sound simultaneously
12621275
while( miditracklen > 0 ) {
1276+
if (mm.err) break;
12631277
miditracklen -= mid_read_delta(h);
12641278
midibyte[0] = mid_read_byte(h);
12651279
miditracklen--;
@@ -1378,6 +1392,7 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
13781392
t, (long)(h->tracktime), midibyte[0]);
13791393
while( midibyte[0] != 0xf7 ) {
13801394
midibyte[0] = mid_read_byte(h);
1395+
if (mm.err) break;
13811396
miditracklen--;
13821397
if( h->debug ) printf(" %02X", midibyte[0]);
13831398
}
@@ -1399,6 +1414,7 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
13991414
t, (long)(h->tracktime), metalen);
14001415
while( metalen > 0 ) {
14011416
midibyte[1] = mid_read_byte(h);
1417+
if (mm.err) break;
14021418
metalen--;
14031419
miditracklen--;
14041420
if( h->debug ) printf(" %02X", midibyte[1]);
@@ -1411,13 +1427,14 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
14111427
metalen = h->deltatime;
14121428
if( metalen > 31 ) metalen = 31;
14131429
if( metalen ) {
1414-
mmreadSBYTES(buf, metalen, h->mmf);
1430+
if (!mmreadSBYTES(buf, metalen, h->mmf)) break;
14151431
miditracklen -= metalen;
14161432
}
14171433
buf[metalen] = '\0';
14181434
metalen = h->deltatime - metalen;
14191435
while( metalen > 0 ) {
14201436
midibyte[1] = mid_read_byte(h);
1437+
if (mm.err) break;
14211438
metalen--;
14221439
miditracklen--;
14231440
}
@@ -1467,7 +1484,7 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
14671484
}
14681485
if( miditracklen < 1 && (runningstatus != 0xff || midibyte[0] != 0x2f) ) {
14691486
delta = mmftell(h->mmf);
1470-
mmreadSBYTES(buf,4,h->mmf);
1487+
if (!mmreadSBYTES(buf,4,h->mmf)) break;
14711488
buf[4] = '\0';
14721489
if( strcmp(buf,"MTrk") ) {
14731490
miditracklen = 0x7fffffff;
@@ -1545,15 +1562,13 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
15451562
m_dwSongFlags = SONG_LINEARSLIDES;
15461563
m_nMinPeriod = 28 << 2;
15471564
m_nMaxPeriod = 1712 << 3;
1548-
if (m_nChannels == 0)
1549-
return FALSE;
1565+
if (m_nChannels == 0) goto ErrorCleanup;
1566+
15501567
// orderlist
15511568
for(t=0; t < numpats; t++)
15521569
Order[t] = t;
1553-
if( !PAT_Load_Instruments(this) ) {
1554-
avoid_reentry = 0;
1555-
return FALSE;
1556-
}
1570+
if( !PAT_Load_Instruments(this) ) goto ErrorCleanup;
1571+
15571572
// ==============================
15581573
// Load the pattern info now!
15591574
if( MID_ReadPatterns(Patterns, PatternSize, h, numpats, m_nChannels) ) {
@@ -1581,4 +1596,10 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength)
15811596
MID_Cleanup(h); // we dont need it anymore
15821597
avoid_reentry = 0; // it is safe now, I'm finished
15831598
return TRUE;
1599+
1600+
ErrorCleanup:
1601+
MID_Cleanup(h);
1602+
ErrorExit:
1603+
avoid_reentry = 0;
1604+
return FALSE;
15841605
}

0 commit comments

Comments
 (0)