Skip to content

Commit 3f082c5

Browse files
sezerosezero
committed
libmodplug: Fix misc. loader crashes and leaks found by libFuzzer:
Patchset by Alice Rowan: Konstanty/libmodplug#58 * AMF (DSMI): fix out-of-bounds reads caused by missing order list bounds checks. * DBM: fix leaks caused by duplicate instrument chunks being loaded. * FAR: fix out-of-bounds reads due to not correctly bounding the maximum pattern read size. * IT: fix out-of-bounds reads in the IT sample decompressors caused by allowing ITReadBits to read past the end of the buffer. * MED: fix out-of-bounds reads due to a faulty MMD2PLAYSEQ bounds check. * MED: fix out-of-bounds reads due to bad sample bounding. * MED: fix out-of-bounds reads due to bad block name bounding (and potential missing nul terminators). * OKT: fix out-of-bounds reads due to incorrect OKTSAMPLE bounding. * OKT: fix out-of-bounds reads due to bad chunk header and order list bounding. * OKT: fix playback errors caused by skipping the first two orders in the order list. * S3M: fix out-of-bounds reads due to missing order list bounds check. * S3M: fix out-of-bounds reads due to missing offset list bounds check. * S3M: fix out-of-bounds reads due to missing panning table check. * STM: fix pattern leaks and pattern size corruption caused by missing MAX_PATTERNS check. * ULT: fix out-of-bounds reads due to incorrect event bounding. * WAV: fix out-of-bounds reads due to not bounds checking the fmt chunk. * WAV: fix hangs caused by missing chunk length bounds check. * WAV: constify pointers derived from lpStream. * XM: fix out-of-bounds reads due to broken XMSAMPLEHEADER check. * XM: fix out-of-bounds reads due to missing pattern data checks. * XM: fix slow loads caused by bad bounding in instrument/sample loops, add other various missing bounds checks. - Fix AMS loader crash and slow load bugs found by libFuzzer: * AMS: fix AMS out-of-bounds reads due to missing song comments checks. * AMS: fix AMS out-of-bounds reads due to missing order list check. * AMS: fix AMS out-of-bounds reads due to missing pattern/track checks. * AMS: fix AMS2 out-of-bounds reads due to missing/broken instrument and envelope bounds checks. * AMS: fix AMS2 out-of-bounds reads due to missing sample bounds checks. * AMS: fix ReadSample out-of-bounds reads due to overflow in packed size bounds check. * AMS: fix AMSUnpack out-of-bounds reads due to missing RLE unpacking bounds checks. * AMS: reduce AMSUnpack slow loads by rejecting samples with truncated or invalid RLE. * AMS: reduce AMSUnpack slow loads by shrinking samples if their packed size couldn't possibly store them. * AMS: constify pointers derived from lpStream. - Fix DMF loader crash/hang/slow load bugs found by libFuzzer: * DMF: fix faulty bounds checks for INFO, SEQU, and SMPI chunks. * DMF: add numerous missing bounds checks for patterns and track data. * DMF: fix out-of-bounds reads caused by missing sample bounds check. * DMF: fix hangs caused by duplicate PATT chunks. * DMF: fix sample leaks caused by duplicate SMPD chunks. * DMF: fix slow loads caused by missing EOF check in DMFUnpack. * DMF: constify pointers derived from lpStream. - Fix MDL loader crash bugs found by libFuzzer: * MDL: fix out-of-bounds reads due to missing info chunk bounds check. * MDL: fix out-of-bounds reads due to a missing bounds check when loading instruments. * MDL: fix out-of-bounds reads and other bugs due to bad envelope bounding and missing duplicate envelope chunk checks. * MDL: fix out-of-bounds reads due to broken track bounds checks. - Fix MT2 loader crashes and hangs found by libFuzzer: * MT2: fix out-of-bounds reads due to missing nDrumDataLen check. * MT2: fix out-of-bounds reads due to missing pattern/track checks. * MT2: fix out-of-bounds reads due to broken/nonsensical instrument bounds checks. * MT2: fix out-of-bounds reads due to missing sample data length bounds check. * MT2: fix out-of-bounds reads due to bad checks on group structs. * MT2: fix hangs caused by overflows preventing the data chunk size bounds check from working. * MT2: constify pattern data pointer derived from lpStream. - Fix PSM loader crash bugs found by libFuzzer: * PSM: fix out-of-bounds reads due to dereferencing lpStream before any bounds checks. * PSM: fix out-of-bounds reads due to reading pPsmPat.data from the stack instead of the input buffer. * PSM: fix out-of-bounds reads due to invalid samples in patterns. * PSM: fix missing pattern length byte-swapping. * PSM: constify pattern data pointer derived from lpStream.
1 parent 6c09865 commit 3f082c5

16 files changed

Lines changed: 236 additions & 105 deletions

‎src/libmodplug/load_amf.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,8 +299,12 @@ BOOL CSoundFile_ReadAMF(CSoundFile *_this, LPCBYTE lpStream, const DWORD dwMemLe
299299
_this->PatternSize[iOrd] = 64;
300300
if (pfh->version >= 14)
301301
{
302+
if (dwMemPos + _this->m_nChannels * sizeof(USHORT) + 2 > dwMemLength) return FALSE;
302303
_this->PatternSize[iOrd] = bswapLE16(*(USHORT *)(lpStream+dwMemPos));
303304
dwMemPos += 2;
305+
} else
306+
{
307+
if (dwMemPos + _this->m_nChannels * sizeof(USHORT) > dwMemLength) return FALSE;
304308
}
305309
ptracks[iOrd] = (USHORT *)(lpStream+dwMemPos);
306310
dwMemPos += _this->m_nChannels * sizeof(USHORT);

‎src/libmodplug/load_ams.c‎

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,16 @@ typedef struct AMSSAMPLEHEADER
3636

3737
#pragma pack()
3838

39+
static BOOL AMSUnpackCheck(const BYTE *lpStream, DWORD dwMemLength, MODINSTRUMENT *ins);
3940

4041
BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
4142
//-----------------------------------------------------------
4243
{
4344
// BYTE pkinf[MAX_SAMPLES];
44-
AMSFILEHEADER *pfh = (AMSFILEHEADER *)lpStream;
45+
const AMSFILEHEADER *pfh = (AMSFILEHEADER *)lpStream;
4546
DWORD dwMemPos;
4647
UINT tmp, tmp2;
47-
48+
4849
if ((!lpStream) || (dwMemLength < 1024)) return FALSE;
4950
if ((pfh->verhi != 0x01) || (SDL_strncmp(pfh->szHeader, "Extreme", 7))
5051
|| (!pfh->patterns) || (!pfh->orders) || (!pfh->samples) || (pfh->samples >= MAX_SAMPLES)
@@ -60,7 +61,7 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
6061
_this->m_nSamples = pfh->samples;
6162
for (UINT nSmp=1; nSmp<=_this->m_nSamples; nSmp++, dwMemPos += sizeof(AMSSAMPLEHEADER))
6263
{
63-
AMSSAMPLEHEADER *psh = (AMSSAMPLEHEADER *)(lpStream + dwMemPos);
64+
const AMSSAMPLEHEADER *psh = (AMSSAMPLEHEADER *)(lpStream + dwMemPos);
6465
MODINSTRUMENT *pins = &_this->Ins[nSmp];
6566
pins->nLength = psh->length;
6667
pins->nLoopStart = psh->loopstart;
@@ -106,14 +107,16 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
106107
dwMemPos += tmp;
107108
}
108109
// Read Song Comments
110+
if (dwMemPos + 2 > dwMemLength) return TRUE;
109111
tmp = *((WORD *)(lpStream+dwMemPos));
110112
dwMemPos += 2;
111-
if (dwMemPos + tmp >= dwMemLength) return TRUE;
113+
if (tmp >= dwMemLength || dwMemPos > dwMemLength - tmp) return TRUE;
112114
if (tmp)
113115
{
114116
dwMemPos += tmp;
115117
}
116118
// Read Order List
119+
if (2*pfh->orders >= dwMemLength || dwMemPos > dwMemLength - 2*pfh->orders) return TRUE;
117120
for (UINT iOrd=0; iOrd<pfh->orders; iOrd++, dwMemPos += 2)
118121
{
119122
UINT n = *((WORD *)(lpStream+dwMemPos));
@@ -125,7 +128,7 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
125128
if (dwMemPos + 4 >= dwMemLength) return TRUE;
126129
UINT len = *((DWORD *)(lpStream + dwMemPos));
127130
dwMemPos += 4;
128-
if ((len >= dwMemLength) || (dwMemPos + len > dwMemLength)) return TRUE;
131+
if ((len >= dwMemLength) || (dwMemPos > dwMemLength - len)) return TRUE;
129132
_this->PatternSize[iPat] = 64;
130133
MODCOMMAND *m = CSoundFile_AllocatePattern(_this->PatternSize[iPat], _this->m_nChannels);
131134
if (!m) return TRUE;
@@ -141,6 +144,7 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
141144
// Note+Instr
142145
if (!(b0 & 0x40))
143146
{
147+
if (i+1 > len) break;
144148
b2 = p[i++];
145149
if (ch < _this->m_nChannels)
146150
{
@@ -149,6 +153,7 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
149153
}
150154
if (b1 & 0x80)
151155
{
156+
if (i+1 > len) break;
152157
b0 |= 0x40;
153158
b1 = p[i++];
154159
}
@@ -166,6 +171,7 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
166171
}
167172
} else
168173
{
174+
if (i+1 > len) break;
169175
b2 = p[i++];
170176
if (ch < _this->m_nChannels)
171177
{
@@ -208,6 +214,7 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
208214
}
209215
if (b1 & 0x80)
210216
{
217+
if (i+1 > len) break;
211218
b1 = p[i++];
212219
if (i <= len) goto anothercommand;
213220
}
@@ -225,7 +232,8 @@ BOOL CSoundFile_ReadAMS(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
225232
{
226233
if (dwMemPos >= dwMemLength - 9) return TRUE;
227234
UINT flags = (_this->Ins[iSmp].uFlags & CHN_16BIT) ? RS_AMS16 : RS_AMS8;
228-
dwMemPos += CSoundFile_ReadSample(_this, &_this->Ins[iSmp], flags, (LPSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos);
235+
if (!AMSUnpackCheck(lpStream+dwMemPos, dwMemLength-dwMemPos, &_this->Ins[iSmp])) break;
236+
dwMemPos += CSoundFile_ReadSample(_this, &_this->Ins[iSmp], flags, (LPCSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos);
229237
}
230238
return TRUE;
231239
}
@@ -288,15 +296,14 @@ typedef struct AMS2SAMPLE
288296
BYTE flags;
289297
} AMS2SAMPLE;
290298

291-
292299
#pragma pack()
293300

294301

295302
BOOL CSoundFile_ReadAMS2(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
296303
//------------------------------------------------------------
297304
{
298305
const AMS2FILEHEADER *pfh = (AMS2FILEHEADER *)lpStream;
299-
AMS2SONGHEADER *psh;
306+
const AMS2SONGHEADER *psh;
300307
DWORD dwMemPos;
301308
BYTE smpmap[16];
302309
BYTE packedsamples[MAX_SAMPLES];
@@ -317,18 +324,22 @@ BOOL CSoundFile_ReadAMS2(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
317324
if (psh->flags & 0x40) _this->m_dwSongFlags |= SONG_LINEARSLIDES;
318325
for (UINT nIns=1; nIns<=_this->m_nInstruments; nIns++)
319326
{
327+
if (dwMemPos >= dwMemLength) return TRUE;
320328
const UINT insnamelen = lpStream[dwMemPos];
321329
dwMemPos += insnamelen + 1;
322-
AMS2INSTRUMENT *pins = (AMS2INSTRUMENT *)(lpStream + dwMemPos);
330+
const AMS2INSTRUMENT *pins = (AMS2INSTRUMENT *)(lpStream + dwMemPos);
323331
dwMemPos += sizeof(AMS2INSTRUMENT);
324-
if (dwMemPos + 1024 >= dwMemLength) return TRUE;
325-
AMS2ENVELOPE *volenv, *panenv, *pitchenv;
332+
const AMS2ENVELOPE *volenv, *panenv, *pitchenv;
333+
if (dwMemPos + sizeof(AMS2ENVELOPE) > dwMemLength) return TRUE;
326334
volenv = (AMS2ENVELOPE *)(lpStream+dwMemPos);
327335
dwMemPos += 5 + volenv->points*3;
336+
if (dwMemPos + sizeof(AMS2ENVELOPE) > dwMemLength) return TRUE;
328337
panenv = (AMS2ENVELOPE *)(lpStream+dwMemPos);
329338
dwMemPos += 5 + panenv->points*3;
339+
if (dwMemPos + sizeof(AMS2ENVELOPE) > dwMemLength) return TRUE;
330340
pitchenv = (AMS2ENVELOPE *)(lpStream+dwMemPos);
331341
dwMemPos += 5 + pitchenv->points*3;
342+
if (dwMemPos >= dwMemLength) return TRUE;
332343
INSTRUMENTHEADER *penv = (INSTRUMENTHEADER *) SDL_calloc(1, sizeof (INSTRUMENTHEADER));
333344
if (!penv) return TRUE;
334345
SDL_memset(smpmap, 0, sizeof(smpmap));
@@ -361,6 +372,7 @@ BOOL CSoundFile_ReadAMS2(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
361372
penv->VolPoints[i] = (WORD)pos;
362373
}
363374
}
375+
if (dwMemPos + 5 > dwMemLength) return TRUE;
364376
penv->nFadeOut = (((lpStream[dwMemPos+2] & 0x0F) << 8) | (lpStream[dwMemPos+1])) << 3;
365377
UINT envflags = lpStream[dwMemPos+3];
366378
if (envflags & 0x01) penv->dwFlags |= ENV_VOLLOOP;
@@ -370,12 +382,15 @@ BOOL CSoundFile_ReadAMS2(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
370382
// Read Samples
371383
for (UINT ismp=0; ismp<pins->samples; ismp++)
372384
{
385+
if (dwMemPos + 1 > dwMemLength) return TRUE;
373386
MODINSTRUMENT *psmp = ((ismp < 16) && (smpmap[ismp])) ? &_this->Ins[smpmap[ismp]] : NULL;
374387
const UINT smpnamelen = lpStream[dwMemPos];
388+
if (dwMemPos + smpnamelen + 1 > dwMemLength) return TRUE;
375389
dwMemPos += smpnamelen + 1;
390+
if (dwMemPos + sizeof(AMS2SAMPLE) > dwMemLength) return TRUE;
376391
if (psmp)
377392
{
378-
AMS2SAMPLE *pams = (AMS2SAMPLE *)(lpStream+dwMemPos);
393+
const AMS2SAMPLE *pams = (AMS2SAMPLE *)(lpStream+dwMemPos);
379394
psmp->nGlobalVol = 64;
380395
psmp->nPan = 128;
381396
psmp->nLength = pams->length;
@@ -502,24 +517,48 @@ BOOL CSoundFile_ReadAMS2(CSoundFile *_this, LPCBYTE lpStream, DWORD dwMemLength)
502517
if (packedsamples[iSmp] & 0x03)
503518
{
504519
flags = (_this->Ins[iSmp].uFlags & CHN_16BIT) ? RS_AMS16 : RS_AMS8;
520+
if (!AMSUnpackCheck(lpStream+dwMemPos, dwMemLength-dwMemPos, &_this->Ins[iSmp])) break;
505521
} else
506522
{
507523
flags = (_this->Ins[iSmp].uFlags & CHN_16BIT) ? RS_PCM16S : RS_PCM8S;
508524
}
509-
dwMemPos += CSoundFile_ReadSample(_this, &_this->Ins[iSmp], flags, (LPSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos);
525+
dwMemPos += CSoundFile_ReadSample(_this, &_this->Ins[iSmp], flags, (LPCSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos);
510526
}
511527
return TRUE;
512528
}
513529

514530

531+
// Precheck AMS packed sample size to determine whether or not it could fit the actual size.
532+
static BOOL AMSUnpackCheck(const BYTE *lpStream, DWORD dwMemLength, MODINSTRUMENT *ins)
533+
// -----------------------------------------------------------------------------------
534+
{
535+
if (dwMemLength < 9) return FALSE;
536+
DWORD packedbytes = *((DWORD *)(lpStream + 4));
537+
538+
DWORD samplebytes = ins->nLength;
539+
if (samplebytes > MAX_SAMPLE_LENGTH) samplebytes = MAX_SAMPLE_LENGTH;
540+
if (ins->uFlags & CHN_16BIT) samplebytes *= 2;
541+
542+
// RLE can pack a run of up to 255 bytes into 3 bytes.
543+
DWORD packedmin = (samplebytes * 3) >> 8;
544+
if (packedbytes < packedmin)
545+
{
546+
samplebytes = packedbytes * (255 / 3) + 2;
547+
ins->nLength = samplebytes;
548+
if (ins->uFlags & CHN_16BIT) ins->nLength >>= 1;
549+
}
550+
551+
return TRUE;
552+
}
553+
515554
/////////////////////////////////////////////////////////////////////
516555
// AMS Sample unpacking
517556

518557
void AMSUnpack(const char *psrc, UINT inputlen, char *pdest, UINT dmax, char packcharacter)
519558
{
520559
UINT tmplen = dmax;
521560
signed char *amstmp = (signed char *) SDL_malloc(tmplen);
522-
561+
523562
if (!amstmp) return;
524563
// Unpack Loop
525564
{
@@ -530,9 +569,11 @@ void AMSUnpack(const char *psrc, UINT inputlen, char *pdest, UINT dmax, char pac
530569
signed char ch = psrc[i++];
531570
if (ch == packcharacter)
532571
{
572+
if (i >= inputlen) break;
533573
BYTE ch2 = psrc[i++];
534574
if (ch2)
535575
{
576+
if (i >= inputlen) break;
536577
ch = psrc[i++];
537578
while (ch2--)
538579
{
@@ -542,6 +583,12 @@ void AMSUnpack(const char *psrc, UINT inputlen, char *pdest, UINT dmax, char pac
542583
} else p[j++] = packcharacter;
543584
} else p[j++] = ch;
544585
}
586+
if (j < tmplen)
587+
{
588+
// Truncated or invalid; don't try to unpack this.
589+
SDL_free(amstmp);
590+
return;
591+
}
545592
}
546593
// Bit Unpack Loop
547594
{

‎src/libmodplug/load_dbm.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ BOOL CSoundFile_ReadDBM(CSoundFile *_this, const BYTE *lpStream, DWORD dwMemLeng
131131
// Instruments
132132
if (chunk_id == bswapLE32(DBM_ID_INST))
133133
{
134+
// Skip duplicate chunks.
135+
if (_this->m_nInstruments) continue;
136+
134137
if (nInstruments >= MAX_INSTRUMENTS) nInstruments = MAX_INSTRUMENTS-1;
135138
for (UINT iIns=0; iIns<nInstruments; iIns++)
136139
{
@@ -227,6 +230,9 @@ BOOL CSoundFile_ReadDBM(CSoundFile *_this, const BYTE *lpStream, DWORD dwMemLeng
227230
DWORD pksize;
228231
UINT nRows;
229232

233+
// Skip duplicate chunks.
234+
if (_this->Patterns[iPat]) break;
235+
230236
if (chunk_pos + sizeof(DBMPATTERN) > dwMemPos) break;
231237
pph = (DBMPATTERN *)(lpStream+chunk_pos);
232238
pksize = bswapBE32(pph->packedsize);

0 commit comments

Comments
 (0)