From 2f8890cb73ecbb6a379b6c05b8feccacf80967c3 Mon Sep 17 00:00:00 2001 From: AliceLR Date: Tue, 22 Jun 2021 19:19:25 -0600 Subject: [PATCH 1/8] Fix misc. loader crashes and leaks found by libFuzzer. * 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. --- src/load_amf.cpp | 4 ++++ src/load_dbm.cpp | 6 ++++++ src/load_far.cpp | 4 ++-- src/load_it.cpp | 21 +++++++++++++-------- src/load_med.cpp | 19 +++++++++++++------ src/load_okt.cpp | 15 +++++++++------ src/load_s3m.cpp | 5 +++++ src/load_stm.cpp | 1 + src/load_ult.cpp | 3 ++- src/load_wav.cpp | 9 +++++---- src/load_xm.cpp | 30 ++++++++++++++++++------------ 11 files changed, 78 insertions(+), 39 deletions(-) diff --git a/src/load_amf.cpp b/src/load_amf.cpp index c560a294..b84be376 100644 --- a/src/load_amf.cpp +++ b/src/load_amf.cpp @@ -317,8 +317,12 @@ BOOL CSoundFile::ReadAMF(LPCBYTE lpStream, const DWORD dwMemLength) PatternSize[iOrd] = 64; if (pfh->version >= 14) { + if (dwMemPos + m_nChannels * sizeof(USHORT) + 2 > dwMemLength) return FALSE; PatternSize[iOrd] = bswapLE16(*(USHORT *)(lpStream+dwMemPos)); dwMemPos += 2; + } else + { + if (dwMemPos + m_nChannels * sizeof(USHORT) > dwMemLength) return FALSE; } ptracks[iOrd] = (USHORT *)(lpStream+dwMemPos); dwMemPos += m_nChannels * sizeof(USHORT); diff --git a/src/load_dbm.cpp b/src/load_dbm.cpp index 9aff94b8..286dcb42 100644 --- a/src/load_dbm.cpp +++ b/src/load_dbm.cpp @@ -136,6 +136,9 @@ BOOL CSoundFile::ReadDBM(const BYTE *lpStream, DWORD dwMemLength) // Instruments if (chunk_id == bswapLE32(DBM_ID_INST)) { + // Skip duplicate chunks. + if (m_nInstruments) continue; + if (nInstruments >= MAX_INSTRUMENTS) nInstruments = MAX_INSTRUMENTS-1; for (UINT iIns=0; iIns dwMemPos) break; pph = (DBMPATTERN *)(lpStream+chunk_pos); pksize = bswapBE32(pph->packedsize); diff --git a/src/load_far.cpp b/src/load_far.cpp index b070a243..d21d1d17 100644 --- a/src/load_far.cpp +++ b/src/load_far.cpp @@ -125,6 +125,7 @@ BOOL CSoundFile::ReadFAR(const BYTE *lpStream, DWORD dwMemLength) continue; } if (dwMemPos + patlen >= dwMemLength) return TRUE; + UINT max = (patlen - 2) & ~3; UINT rows = (patlen - 2) >> 6; if (!rows) { @@ -133,13 +134,12 @@ BOOL CSoundFile::ReadFAR(const BYTE *lpStream, DWORD dwMemLength) } if (rows > 256) rows = 256; if (rows < 16) rows = 16; + if (max > rows*16*4) max = rows*16*4; PatternSize[ipat] = rows; if ((Patterns[ipat] = AllocatePattern(rows, m_nChannels)) == NULL) return TRUE; MODCOMMAND *m = Patterns[ipat]; UINT patbrk = lpStream[dwMemPos]; const BYTE *p = lpStream + dwMemPos + 2; - UINT max = rows*16*4; - if (max > patlen-2) max = patlen-2; for (UINT len=0; len= ibufend) + return 0; + bitbuf = *ibuf++; bitnum = 8; } @@ -1218,6 +1221,7 @@ void ITUnpack8Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dwM { signed char *pDst = pSample; LPBYTE pSrc = lpMemFile; + LPBYTE pStop = lpMemFile + dwMemLength; DWORD wHdr = 0; DWORD wCount = 0; DWORD bitbuf = 0; @@ -1241,13 +1245,13 @@ void ITUnpack8Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dwM DWORD dwPos = 0; do { - WORD wBits = (WORD)ITReadBits(bitbuf, bitnum, pSrc, bLeft); + WORD wBits = (WORD)ITReadBits(bitbuf, bitnum, pSrc, pStop, bLeft); if (bLeft < 7) { DWORD i = 1 << (bLeft-1); DWORD j = wBits & 0xFFFF; if (i != j) goto UnpackByte; - wBits = (WORD)(ITReadBits(bitbuf, bitnum, pSrc, 3) + 1) & 0xFF; + wBits = (WORD)(ITReadBits(bitbuf, bitnum, pSrc, pStop, 3) + 1) & 0xFF; bLeft = ((BYTE)wBits < bLeft) ? (BYTE)wBits : (BYTE)((wBits+1) & 0xFF); goto Next; } @@ -1285,7 +1289,7 @@ void ITUnpack8Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dwM SkipByte: dwPos++; Next: - if (pSrc >= lpMemFile+dwMemLength+1) return; + if (pSrc >= pStop + 1) return; } while (dwPos < d); // Move On wCount -= d; @@ -1300,6 +1304,7 @@ void ITUnpack16Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dw { signed short *pDst = (signed short *)pSample; LPBYTE pSrc = lpMemFile; + LPBYTE pStop = lpMemFile + dwMemLength; DWORD wHdr = 0; DWORD wCount = 0; DWORD bitbuf = 0; @@ -1324,13 +1329,13 @@ void ITUnpack16Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dw DWORD dwPos = 0; do { - DWORD dwBits = ITReadBits(bitbuf, bitnum, pSrc, bLeft); + DWORD dwBits = ITReadBits(bitbuf, bitnum, pSrc, pStop, bLeft); if (bLeft < 7) { DWORD i = 1 << (bLeft-1); DWORD j = dwBits; if (i != j) goto UnpackByte; - dwBits = ITReadBits(bitbuf, bitnum, pSrc, 4) + 1; + dwBits = ITReadBits(bitbuf, bitnum, pSrc, pStop, 4) + 1; bLeft = ((BYTE)(dwBits & 0xFF) < bLeft) ? (BYTE)(dwBits & 0xFF) : (BYTE)((dwBits+1) & 0xFF); goto Next; } @@ -1368,13 +1373,13 @@ void ITUnpack16Bit(signed char *pSample, DWORD dwLen, LPBYTE lpMemFile, DWORD dw SkipByte: dwPos++; Next: - if (pSrc >= lpMemFile+dwMemLength+1) return; + if (pSrc >= pStop + 1) return; } while (dwPos < d); // Move On wCount -= d; dwLen -= d; pDst += d; - if (pSrc >= lpMemFile+dwMemLength) break; + if (pSrc >= pStop) break; } } diff --git a/src/load_med.cpp b/src/load_med.cpp index 745e2c65..36e03583 100644 --- a/src/load_med.cpp +++ b/src/load_med.cpp @@ -662,7 +662,7 @@ BOOL CSoundFile::ReadMed(const BYTE *lpStream, DWORD dwMemLength) } UINT pseq = 0; - if ((playseqtable) && (playseqtable < dwMemLength) && (nplayseq*4 < dwMemLength - playseqtable)) + if ((playseqtable) && (playseqtable < dwMemLength) && (nplayseq*4 + 4 < dwMemLength - playseqtable)) { pseq = bswapBE32(((LPDWORD)(lpStream+playseqtable))[nplayseq]); } @@ -789,10 +789,11 @@ BOOL CSoundFile::ReadMed(const BYTE *lpStream, DWORD dwMemLength) #endif if ((len > MAX_SAMPLE_LENGTH) || (dwPos + len + 6 > dwMemLength)) len = 0; UINT flags = RS_PCM8S, stype = bswapBE16(psdh->type); - LPSTR psdata = (LPSTR)(lpStream + dwPos + 6); + dwPos += 6; if (stype & 0x80) { - psdata += (stype & 0x20) ? 14 : 6; + dwPos += (stype & 0x20) ? 14 : 6; + if (dwPos >= dwMemLength) continue; } else { if (stype & 0x10) @@ -807,7 +808,7 @@ BOOL CSoundFile::ReadMed(const BYTE *lpStream, DWORD dwMemLength) if (stype & 0x20) len /= 2; } Ins[iSmp+1].nLength = len; - ReadSample(&Ins[iSmp+1], flags, psdata, dwMemLength - dwPos - 6); + ReadSample(&Ins[iSmp+1], flags, (const char *)(lpStream + dwPos), dwMemLength - dwPos); } // Reading patterns (blocks) if (wNumBlocks > MAX_PATTERNS) wNumBlocks = MAX_PATTERNS; @@ -876,9 +877,15 @@ BOOL CSoundFile::ReadMed(const BYTE *lpStream, DWORD dwMemLength) { DWORD nameofs = bswapBE32(pbi->blockname); UINT namelen = bswapBE32(pbi->blocknamelen); - if ((nameofs < dwMemLength) && (namelen < dwMemLength + nameofs)) + if ((namelen < dwMemLength) && (nameofs < dwMemLength - namelen)) { - SetPatternName(iBlk, (LPCSTR)(lpStream+nameofs)); + // SetPatternName expects a nul-terminated string. + char blockname[MAX_PATTERNNAME]; + if (namelen >= MAX_PATTERNNAME) namelen = MAX_PATTERNNAME - 1; + memcpy(blockname, lpStream + nameofs, namelen); + blockname[namelen] = '\0'; + + SetPatternName(iBlk, blockname); } } if (pbi->cmdexttable) diff --git a/src/load_okt.cpp b/src/load_okt.cpp index 7968f0b1..b0cbb004 100644 --- a/src/load_okt.cpp +++ b/src/load_okt.cpp @@ -59,7 +59,7 @@ BOOL CSoundFile::ReadOKT(const BYTE *lpStream, DWORD dwMemLength) // Reading samples for (UINT smp=1; smp <= nsamples; smp++) { - if (dwMemPos >= dwMemLength) return TRUE; + if (dwMemPos + sizeof(OKTSAMPLE) >= dwMemLength) return TRUE; if (smp < MAX_SAMPLES) { OKTSAMPLE *psmp = (OKTSAMPLE *)(lpStream + dwMemPos); @@ -78,33 +78,36 @@ BOOL CSoundFile::ReadOKT(const BYTE *lpStream, DWORD dwMemLength) dwMemPos += sizeof(OKTSAMPLE); } // SPEE - if (dwMemPos >= dwMemLength) return TRUE; + if (dwMemPos + 10 > dwMemLength) return TRUE; if (*((DWORD *)(lpStream + dwMemPos)) == 0x45455053) { m_nDefaultSpeed = lpStream[dwMemPos+9]; dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8; } // SLEN - if (dwMemPos >= dwMemLength) return TRUE; + if (dwMemPos + 10 > dwMemLength) return TRUE; if (*((DWORD *)(lpStream + dwMemPos)) == 0x4E454C53) { + if (dwMemPos + 10 > dwMemLength) return TRUE; npatterns = lpStream[dwMemPos+9]; dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8; } // PLEN - if (dwMemPos >= dwMemLength) return TRUE; + if (dwMemPos + 10 > dwMemLength) return TRUE; if (*((DWORD *)(lpStream + dwMemPos)) == 0x4E454C50) { + if (dwMemPos + 10 > dwMemLength) return TRUE; norders = lpStream[dwMemPos+9]; dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8; } // PATT - if (dwMemPos >= dwMemLength) return TRUE; + if (dwMemPos + 8 > dwMemLength) return TRUE; if (*((DWORD *)(lpStream + dwMemPos)) == 0x54544150) { UINT orderlen = norders; if (orderlen >= MAX_ORDERS) orderlen = MAX_ORDERS-1; - for (UINT i=0; i dwMemLength) return TRUE; + for (UINT i=0; i1; j--) { if (Order[j-1]) break; Order[j-1] = 0xFF; } dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8; } diff --git a/src/load_s3m.cpp b/src/load_s3m.cpp index ef61f601..9553e361 100644 --- a/src/load_s3m.cpp +++ b/src/load_s3m.cpp @@ -254,6 +254,7 @@ BOOL CSoundFile::ReadS3M(const BYTE *lpStream, DWORD dwMemLength) if (iord > MAX_ORDERS) iord = MAX_ORDERS; if (iord) { + if (dwMemPos + iord > dwMemLength) return FALSE; memcpy(Order, lpStream+dwMemPos, iord); dwMemPos += iord; } @@ -271,6 +272,8 @@ BOOL CSoundFile::ReadS3M(const BYTE *lpStream, DWORD dwMemLength) if (nins+npat) { + if (2*(nins+npat) + dwMemPos > dwMemLength) return FALSE; + memcpy(ptr, lpStream+dwMemPos, 2*(nins+npat)); dwMemPos += 2*(nins+npat); for (UINT j = 0; j < (nins+npat); ++j) { @@ -278,6 +281,8 @@ BOOL CSoundFile::ReadS3M(const BYTE *lpStream, DWORD dwMemLength) } if (psfh.panning_present == 252) { + if (dwMemPos + 32 > dwMemLength) return FALSE; + const BYTE *chnpan = lpStream+dwMemPos; for (UINT i=0; i<32; i++) if (chnpan[i] & 0x20) { diff --git a/src/load_stm.cpp b/src/load_stm.cpp index 6f55b78e..5502c480 100644 --- a/src/load_stm.cpp +++ b/src/load_stm.cpp @@ -107,6 +107,7 @@ BOOL CSoundFile::ReadSTM(const BYTE *lpStream, DWORD dwMemLength) dwMemPos = sizeof(STMHEADER); for (UINT nOrd=0; nOrd= 99) Order[nOrd] = 0xFF; UINT nPatterns = phdr->numpat; + if (nPatterns > MAX_PATTERNS) nPatterns = MAX_PATTERNS; for (UINT nPat=0; nPat dwMemLength) return TRUE; diff --git a/src/load_ult.cpp b/src/load_ult.cpp index 83024f21..26067ca3 100644 --- a/src/load_ult.cpp +++ b/src/load_ult.cpp @@ -155,11 +155,12 @@ BOOL CSoundFile::ReadUlt(const BYTE *lpStream, DWORD dwMemLength) UINT row = 0; while (row < 64) { - if (dwMemPos + 6 > dwMemLength) return TRUE; + if (dwMemPos + 5 > dwMemLength) return TRUE; UINT rep = 1; UINT note = lpStream[dwMemPos++]; if (note == 0xFC) { + if (dwMemPos + 7 > dwMemLength) return TRUE; rep = lpStream[dwMemPos]; note = lpStream[dwMemPos+1]; dwMemPos += 2; diff --git a/src/load_wav.cpp b/src/load_wav.cpp index cf721df3..48de6137 100644 --- a/src/load_wav.cpp +++ b/src/load_wav.cpp @@ -18,9 +18,9 @@ BOOL CSoundFile::ReadWav(const BYTE *lpStream, DWORD dwMemLength) //--------------------------------------------------------------- { DWORD dwMemPos = 0; - WAVEFILEHEADER *phdr = (WAVEFILEHEADER *)lpStream; - WAVEFORMATHEADER *pfmt = (WAVEFORMATHEADER *)(lpStream + sizeof(WAVEFILEHEADER)); - if ((!lpStream) || (dwMemLength < (DWORD)sizeof(WAVEFILEHEADER))) return FALSE; + const WAVEFILEHEADER *phdr = (WAVEFILEHEADER *)lpStream; + const WAVEFORMATHEADER *pfmt = (WAVEFORMATHEADER *)(lpStream + sizeof(WAVEFILEHEADER)); + if ((!lpStream) || (dwMemLength < sizeof(WAVEFILEHEADER)+sizeof(WAVEFORMATHEADER))) return FALSE; if ((phdr->id_RIFF != IFFID_RIFF) || (phdr->id_WAVE != IFFID_WAVE) || (pfmt->id_fmt != IFFID_fmt)) return FALSE; dwMemPos = sizeof(WAVEFILEHEADER) + 8 + pfmt->hdrlen; @@ -32,11 +32,12 @@ BOOL CSoundFile::ReadWav(const BYTE *lpStream, DWORD dwMemLength) || (pfmt->bitspersample & 7) || (pfmt->bitspersample < 8) || (pfmt->bitspersample > 32)) return FALSE; - WAVEDATAHEADER *pdata; + const WAVEDATAHEADER *pdata; for (;;) { pdata = (WAVEDATAHEADER *)(lpStream + dwMemPos); if (pdata->id_data == IFFID_data) break; + if (pdata->length >= dwMemLength || dwMemPos > dwMemLength - pdata->length) return FALSE; dwMemPos += pdata->length + 8; if (dwMemPos >= dwMemLength - 8) return FALSE; } diff --git a/src/load_xm.cpp b/src/load_xm.cpp index 3a22b7da..b3fb4357 100644 --- a/src/load_xm.cpp +++ b/src/load_xm.cpp @@ -193,13 +193,14 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) UINT vol = 0; if (b & 0x80) { - if (b & 1) p->note = src[j++]; - if (b & 2) p->instr = src[j++]; - if (b & 4) vol = src[j++]; - if (b & 8) p->command = src[j++]; - if (b & 16) p->param = src[j++]; + if ((b & 1) && j < packsize) p->note = src[j++]; + if ((b & 2) && j < packsize) p->instr = src[j++]; + if ((b & 4) && j < packsize) vol = src[j++]; + if ((b & 8) && j < packsize) p->command = src[j++]; + if ((b & 16) && j < packsize) p->param = src[j++]; } else { + if (j + 5 > packsize) break; p->note = b; p->instr = src[j++]; vol = src[j++]; @@ -281,16 +282,18 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) DWORD samplesize[32]; UINT samplemap[32]; WORD nsamples; + DWORD pihlen; if (dwMemPos + sizeof(XMINSTRUMENTHEADER) >= dwMemLength) return TRUE; pih = (XMINSTRUMENTHEADER *)(lpStream+dwMemPos); - if (dwMemPos + bswapLE32(pih->size) > dwMemLength) return TRUE; + pihlen = bswapLE32(pih->size); + if (pihlen >= dwMemLength || dwMemPos > dwMemLength - pihlen) return TRUE; if ((Headers[iIns] = new INSTRUMENTHEADER) == NULL) continue; memset(Headers[iIns], 0, sizeof(INSTRUMENTHEADER)); memcpy(Headers[iIns]->name, pih->name, 22); if ((nsamples = pih->samples) > 0) { - if (dwMemPos + sizeof(XMSAMPLEHEADER) > dwMemLength) return TRUE; + if (dwMemPos + sizeof(XMINSTRUMENTHEADER) + sizeof(XMSAMPLEHEADER) > dwMemLength) return TRUE; memcpy(&xmsh, lpStream+dwMemPos+sizeof(XMINSTRUMENTHEADER), sizeof(XMSAMPLEHEADER)); xmsh.shsize = bswapLE32(xmsh.shsize); for (int i = 0; i < 24; ++i) { @@ -299,10 +302,10 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) } xmsh.volfade = bswapLE16(xmsh.volfade); xmsh.res = bswapLE16(xmsh.res); - dwMemPos += bswapLE32(pih->size); + dwMemPos += pihlen; } else { - if (bswapLE32(pih->size)) dwMemPos += bswapLE32(pih->size); + if (pihlen) dwMemPos += pihlen; else dwMemPos += sizeof(XMINSTRUMENTHEADER); continue; } @@ -439,7 +442,7 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) for (UINT ins=0; ins dwMemLength) - || (dwMemPos + xmsh.shsize > dwMemLength)) return TRUE; + || (xmsh.shsize >= dwMemLength) || (dwMemPos > dwMemLength - xmsh.shsize)) return TRUE; memcpy(&xmss, lpStream+dwMemPos, sizeof(xmss)); xmss.samplen = bswapLE32(xmss.samplen); xmss.loopstart = bswapLE32(xmss.loopstart); @@ -536,6 +539,7 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) { UINT len = *((DWORD *)(lpStream+dwMemPos+4)); dwMemPos += 8; + if (len >= dwMemLength || dwMemPos > dwMemLength - len) return TRUE; if (len == sizeof(MODMIDICFG)) { memcpy(&m_MidiCfg, lpStream+dwMemPos, len); @@ -547,7 +551,8 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) { UINT len = *((DWORD *)(lpStream+dwMemPos+4)); dwMemPos += 8; - if ((dwMemPos + len <= dwMemLength) && (len <= MAX_PATTERNS*MAX_PATTERNNAME) && (len >= MAX_PATTERNNAME)) + if (len >= dwMemLength || dwMemPos > dwMemLength - len) return TRUE; + if ((len <= MAX_PATTERNS*MAX_PATTERNNAME) && (len >= MAX_PATTERNNAME)) { m_lpszPatternNames = new char[len]; @@ -564,7 +569,8 @@ BOOL CSoundFile::ReadXM(const BYTE *lpStream, DWORD dwMemLength) { UINT len = *((DWORD *)(lpStream+dwMemPos+4)); dwMemPos += 8; - if ((dwMemPos + len <= dwMemLength) && (len <= MAX_BASECHANNELS*MAX_CHANNELNAME)) + if (len >= dwMemLength || dwMemPos > dwMemLength - len) return TRUE; + if (len <= MAX_BASECHANNELS*MAX_CHANNELNAME) { UINT n = len / MAX_CHANNELNAME; for (UINT i=0; i Date: Tue, 22 Jun 2021 19:22:20 -0600 Subject: [PATCH 2/8] 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. --- src/load_ams.cpp | 77 +++++++++++++++++++++++++++++++++++++++--------- src/sndfile.cpp | 7 +++-- 2 files changed, 67 insertions(+), 17 deletions(-) diff --git a/src/load_ams.cpp b/src/load_ams.cpp index b05e5ab2..f2798bae 100644 --- a/src/load_ams.cpp +++ b/src/load_ams.cpp @@ -41,15 +41,16 @@ typedef struct AMSSAMPLEHEADER #pragma pack() +static BOOL AMSUnpackCheck(const BYTE *lpStream, DWORD dwMemLength, MODINSTRUMENT *ins); BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) //----------------------------------------------------------- { BYTE pkinf[MAX_SAMPLES]; - AMSFILEHEADER *pfh = (AMSFILEHEADER *)lpStream; + const AMSFILEHEADER *pfh = (AMSFILEHEADER *)lpStream; DWORD dwMemPos; UINT tmp, tmp2; - + if ((!lpStream) || (dwMemLength < 1024)) return FALSE; if ((pfh->verhi != 0x01) || (strncmp(pfh->szHeader, "Extreme", 7)) || (!pfh->patterns) || (!pfh->orders) || (!pfh->samples) || (pfh->samples >= MAX_SAMPLES) @@ -65,7 +66,7 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) m_nSamples = pfh->samples; for (UINT nSmp=1; nSmp<=m_nSamples; nSmp++, dwMemPos += sizeof(AMSSAMPLEHEADER)) { - AMSSAMPLEHEADER *psh = (AMSSAMPLEHEADER *)(lpStream + dwMemPos); + const AMSSAMPLEHEADER *psh = (AMSSAMPLEHEADER *)(lpStream + dwMemPos); MODINSTRUMENT *pins = &Ins[nSmp]; pins->nLength = psh->length; pins->nLoopStart = psh->loopstart; @@ -117,9 +118,10 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) dwMemPos += tmp; } // Read Song Comments + if (dwMemPos + 2 > dwMemLength) return TRUE; tmp = *((WORD *)(lpStream+dwMemPos)); dwMemPos += 2; - if (dwMemPos + tmp >= dwMemLength) return TRUE; + if (tmp >= dwMemLength || dwMemPos > dwMemLength - tmp) return TRUE; if (tmp) { m_lpszSongComments = new char[tmp+1]; // changed from CHAR @@ -129,6 +131,7 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) dwMemPos += tmp; } // Read Order List + if (2*pfh->orders >= dwMemLength || dwMemPos > dwMemLength - 2*pfh->orders) return TRUE; for (UINT iOrd=0; iOrdorders; iOrd++, dwMemPos += 2) { UINT n = *((WORD *)(lpStream+dwMemPos)); @@ -140,7 +143,7 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) if (dwMemPos + 4 >= dwMemLength) return TRUE; UINT len = *((DWORD *)(lpStream + dwMemPos)); dwMemPos += 4; - if ((len >= dwMemLength) || (dwMemPos + len > dwMemLength)) return TRUE; + if ((len >= dwMemLength) || (dwMemPos > dwMemLength - len)) return TRUE; PatternSize[iPat] = 64; MODCOMMAND *m = AllocatePattern(PatternSize[iPat], m_nChannels); if (!m) return TRUE; @@ -156,6 +159,7 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) // Note+Instr if (!(b0 & 0x40)) { + if (i+1 > len) break; b2 = p[i++]; if (ch < m_nChannels) { @@ -164,6 +168,7 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) } if (b1 & 0x80) { + if (i+1 > len) break; b0 |= 0x40; b1 = p[i++]; } @@ -181,6 +186,7 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) } } else { + if (i+1 > len) break; b2 = p[i++]; if (ch < m_nChannels) { @@ -223,6 +229,7 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) } if (b1 & 0x80) { + if (i+1 > len) break; b1 = p[i++]; if (i <= len) goto anothercommand; } @@ -240,7 +247,8 @@ BOOL CSoundFile::ReadAMS(LPCBYTE lpStream, DWORD dwMemLength) { if (dwMemPos >= dwMemLength - 9) return TRUE; UINT flags = (Ins[iSmp].uFlags & CHN_16BIT) ? RS_AMS16 : RS_AMS8; - dwMemPos += ReadSample(&Ins[iSmp], flags, (LPSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos); + if (!AMSUnpackCheck(lpStream+dwMemPos, dwMemLength-dwMemPos, &Ins[iSmp])) break; + dwMemPos += ReadSample(&Ins[iSmp], flags, (LPCSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos); } return TRUE; } @@ -307,11 +315,12 @@ typedef struct AMS2SAMPLE #pragma pack() + BOOL CSoundFile::ReadAMS2(LPCBYTE lpStream, DWORD dwMemLength) //------------------------------------------------------------ { const AMS2FILEHEADER *pfh = (AMS2FILEHEADER *)lpStream; - AMS2SONGHEADER *psh; + const AMS2SONGHEADER *psh; DWORD dwMemPos; BYTE smpmap[16]; BYTE packedsamples[MAX_SAMPLES]; @@ -337,19 +346,23 @@ BOOL CSoundFile::ReadAMS2(LPCBYTE lpStream, DWORD dwMemLength) if (psh->flags & 0x40) m_dwSongFlags |= SONG_LINEARSLIDES; for (UINT nIns=1; nIns<=m_nInstruments; nIns++) { + if (dwMemPos >= dwMemLength) return TRUE; UINT insnamelen = lpStream[dwMemPos]; - CHAR *pinsname = (CHAR *)(lpStream+dwMemPos+1); + const CHAR *pinsname = (CHAR *)(lpStream+dwMemPos+1); dwMemPos += insnamelen + 1; - AMS2INSTRUMENT *pins = (AMS2INSTRUMENT *)(lpStream + dwMemPos); + const AMS2INSTRUMENT *pins = (AMS2INSTRUMENT *)(lpStream + dwMemPos); dwMemPos += sizeof(AMS2INSTRUMENT); - if (dwMemPos + 1024 >= dwMemLength) return TRUE; - AMS2ENVELOPE *volenv, *panenv, *pitchenv; + const AMS2ENVELOPE *volenv, *panenv, *pitchenv; + if (dwMemPos + sizeof(AMS2ENVELOPE) > dwMemLength) return TRUE; volenv = (AMS2ENVELOPE *)(lpStream+dwMemPos); dwMemPos += 5 + volenv->points*3; + if (dwMemPos + sizeof(AMS2ENVELOPE) > dwMemLength) return TRUE; panenv = (AMS2ENVELOPE *)(lpStream+dwMemPos); dwMemPos += 5 + panenv->points*3; + if (dwMemPos + sizeof(AMS2ENVELOPE) > dwMemLength) return TRUE; pitchenv = (AMS2ENVELOPE *)(lpStream+dwMemPos); dwMemPos += 5 + pitchenv->points*3; + if (dwMemPos >= dwMemLength) return TRUE; INSTRUMENTHEADER *penv = new INSTRUMENTHEADER; if (!penv) return TRUE; memset(smpmap, 0, sizeof(smpmap)); @@ -389,6 +402,7 @@ BOOL CSoundFile::ReadAMS2(LPCBYTE lpStream, DWORD dwMemLength) penv->VolPoints[i] = (WORD)pos; } } + if (dwMemPos + 5 > dwMemLength) return TRUE; penv->nFadeOut = (((lpStream[dwMemPos+2] & 0x0F) << 8) | (lpStream[dwMemPos+1])) << 3; UINT envflags = lpStream[dwMemPos+3]; if (envflags & 0x01) penv->dwFlags |= ENV_VOLLOOP; @@ -398,16 +412,19 @@ BOOL CSoundFile::ReadAMS2(LPCBYTE lpStream, DWORD dwMemLength) // Read Samples for (UINT ismp=0; ismpsamples; ismp++) { + if (dwMemPos + 1 > dwMemLength) return TRUE; MODINSTRUMENT *psmp = ((ismp < 16) && (smpmap[ismp])) ? &Ins[smpmap[ismp]] : NULL; UINT smpnamelen = lpStream[dwMemPos]; + if (dwMemPos + smpnamelen + 1 > dwMemLength) return TRUE; if ((psmp) && (smpnamelen) && (smpnamelen <= 22)) { memcpy(m_szNames[smpmap[ismp]], lpStream+dwMemPos+1, smpnamelen); } dwMemPos += smpnamelen + 1; + if (dwMemPos + sizeof(AMS2SAMPLE) > dwMemLength) return TRUE; if (psmp) { - AMS2SAMPLE *pams = (AMS2SAMPLE *)(lpStream+dwMemPos); + const AMS2SAMPLE *pams = (AMS2SAMPLE *)(lpStream+dwMemPos); psmp->nGlobalVol = 64; psmp->nPan = 128; psmp->nLength = pams->length; @@ -547,16 +564,40 @@ BOOL CSoundFile::ReadAMS2(LPCBYTE lpStream, DWORD dwMemLength) if (packedsamples[iSmp] & 0x03) { flags = (Ins[iSmp].uFlags & CHN_16BIT) ? RS_AMS16 : RS_AMS8; + if (!AMSUnpackCheck(lpStream+dwMemPos, dwMemLength-dwMemPos, &Ins[iSmp])) break; } else { flags = (Ins[iSmp].uFlags & CHN_16BIT) ? RS_PCM16S : RS_PCM8S; } - dwMemPos += ReadSample(&Ins[iSmp], flags, (LPSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos); + dwMemPos += ReadSample(&Ins[iSmp], flags, (LPCSTR)(lpStream+dwMemPos), dwMemLength-dwMemPos); } return TRUE; } +// Precheck AMS packed sample size to determine whether or not it could fit the actual size. +static BOOL AMSUnpackCheck(const BYTE *lpStream, DWORD dwMemLength, MODINSTRUMENT *ins) +// ----------------------------------------------------------------------------------- +{ + if (dwMemLength < 9) return FALSE; + DWORD packedbytes = *((DWORD *)(lpStream + 4)); + + DWORD samplebytes = ins->nLength; + if (samplebytes > MAX_SAMPLE_LENGTH) samplebytes = MAX_SAMPLE_LENGTH; + if (ins->uFlags & CHN_16BIT) samplebytes *= 2; + + // RLE can pack a run of up to 255 bytes into 3 bytes. + DWORD packedmin = (samplebytes * 3) >> 8; + if (packedbytes < packedmin) + { + samplebytes = packedbytes * (255 / 3) + 2; + ins->nLength = samplebytes; + if (ins->uFlags & CHN_16BIT) ins->nLength >>= 1; + } + + return TRUE; +} + ///////////////////////////////////////////////////////////////////// // AMS Sample unpacking @@ -564,7 +605,7 @@ void AMSUnpack(const char *psrc, UINT inputlen, char *pdest, UINT dmax, char pac { UINT tmplen = dmax; signed char *amstmp = new signed char[tmplen]; - + if (!amstmp) return; // Unpack Loop { @@ -575,9 +616,11 @@ void AMSUnpack(const char *psrc, UINT inputlen, char *pdest, UINT dmax, char pac signed char ch = psrc[i++]; if (ch == packcharacter) { + if (i >= inputlen) break; BYTE ch2 = psrc[i++]; if (ch2) { + if (i >= inputlen) break; ch = psrc[i++]; while (ch2--) { @@ -587,6 +630,12 @@ void AMSUnpack(const char *psrc, UINT inputlen, char *pdest, UINT dmax, char pac } else p[j++] = packcharacter; } else p[j++] = ch; } + if (j < tmplen) + { + // Truncated or invalid; don't try to unpack this. + delete[] amstmp; + return; + } } // Bit Unpack Loop { diff --git a/src/sndfile.cpp b/src/sndfile.cpp index 2dfc79e2..5b870a38 100644 --- a/src/sndfile.cpp +++ b/src/sndfile.cpp @@ -1365,11 +1365,12 @@ UINT CSoundFile::ReadSample(MODINSTRUMENT *pIns, UINT nFlags, LPCSTR lpMemFile, { const char *psrc = lpMemFile; char packcharacter = lpMemFile[8], *pdest = (char *)pIns->pSample; - len += bswapLE32(*((LPDWORD)(lpMemFile+4))); - if (len > dwMemLength) len = dwMemLength; + UINT smplen = bswapLE32(*((LPDWORD)(lpMemFile+4))); + if (smplen > dwMemLength - 9) smplen = dwMemLength - 9; + len += smplen; UINT dmax = pIns->nLength; if (pIns->uFlags & CHN_16BIT) dmax <<= 1; - AMSUnpack(psrc+9, len-9, pdest, dmax, packcharacter); + AMSUnpack(psrc+9, smplen, pdest, dmax, packcharacter); } break; From ecfba0dcb9ef812ad62f0e38639a8d7d4b407fb0 Mon Sep 17 00:00:00 2001 From: AliceLR Date: Tue, 22 Jun 2021 19:24:09 -0600 Subject: [PATCH 3/8] 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. --- src/load_dmf.cpp | 71 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 51 insertions(+), 20 deletions(-) diff --git a/src/load_dmf.cpp b/src/load_dmf.cpp index 7bfca226..9eba77df 100644 --- a/src/load_dmf.cpp +++ b/src/load_dmf.cpp @@ -87,11 +87,12 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) //--------------------------------------------------------------- { const DMFHEADER *pfh = (DMFHEADER *)lpStream; - DMFINFO *psi; - DMFSEQU *sequ; + const DMFINFO *psi; + const DMFPATT *patt; + const DMFSEQU *sequ; DWORD dwMemPos; BYTE infobyte[32]; - BYTE smplflags[MAX_SAMPLES], hasSMPI = 0; + BYTE smplflags[MAX_SAMPLES], hasSMPI = 0, hasSMPD = 0; if ((!lpStream) || (dwMemLength < 1024)) return FALSE; if ((pfh->id != 0x464d4444) || (!pfh->version) || (pfh->version & 0xF0)) return FALSE; @@ -115,7 +116,7 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) case 0x47534d43: psi = (DMFINFO *)(lpStream+dwMemPos); if (id == 0x47534d43) dwMemPos++; - if ((psi->infosize > dwMemLength) || (psi->infosize + dwMemPos + 8 > dwMemLength)) goto dmfexit; + if ((psi->infosize > dwMemLength) || (dwMemPos + 8 > dwMemLength - psi->infosize)) goto dmfexit; if ((psi->infosize >= 8) && (!m_lpszSongComments)) { m_lpszSongComments = new char[psi->infosize]; // changed from CHAR @@ -138,9 +139,10 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) // "SEQU" case 0x55514553: sequ = (DMFSEQU *)(lpStream+dwMemPos); - if ((sequ->seqsize >= dwMemLength) || (dwMemPos + sequ->seqsize + 12 > dwMemLength)) goto dmfexit; + if ((sequ->seqsize >= dwMemLength) || (dwMemPos + 8 > dwMemLength - sequ->seqsize)) goto dmfexit; + if (sequ->seqsize >= 4) { - UINT nseq = sequ->seqsize >> 1; + UINT nseq = (sequ->seqsize - 4) >> 1; if (nseq >= MAX_ORDERS-1) nseq = MAX_ORDERS-1; if (sequ->loopstart < nseq) m_nRestartPos = sequ->loopstart; for (UINT i=0; isequ[i]; @@ -150,12 +152,12 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) // "PATT" case 0x54544150: - if (!m_nChannels) + patt = (DMFPATT *)(lpStream+dwMemPos); + if ((patt->patsize >= dwMemLength) || (dwMemPos + 8 > dwMemLength - patt->patsize)) goto dmfexit; + if (patt->patsize >= 4 && !m_nChannels) { - DMFPATT *patt = (DMFPATT *)(lpStream+dwMemPos); UINT numpat; DWORD dwPos = dwMemPos + 11; - if ((patt->patsize >= dwMemLength) || (dwMemPos + patt->patsize + 8 > dwMemLength)) goto dmfexit; numpat = patt->numpat; if (numpat > MAX_PATTERNS) numpat = MAX_PATTERNS; m_nChannels = patt->tracks; @@ -164,7 +166,8 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) if (m_nChannels < 4) m_nChannels = 4; for (UINT npat=0; npat= dwMemLength) break; #ifdef DMFLOG Log("Pattern #%d: %d tracks, %d rows\n", npat, pt->tracks, pt->ticks); #endif @@ -174,7 +177,7 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) if (ticks > 256) ticks = 256; if (ticks < 16) ticks = 16; dwPos += 8; - if ((pt->jmpsize >= dwMemLength) || (dwPos + pt->jmpsize + 4 >= dwMemLength)) break; + if ((pt->jmpsize >= dwMemLength) || (dwPos + 4 > dwMemLength - pt->jmpsize)) break; PatternSize[npat] = (WORD)ticks; MODCOMMAND *m = AllocatePattern(PatternSize[npat], m_nChannels); if (!m) goto dmfexit; @@ -193,6 +196,7 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) // Parse track global effects if (!glbinfobyte) { + if (d+1 > dwPos) break; BYTE info = lpStream[d++]; BYTE infoval = 0; if ((info & 0x80) && (d < dwPos)) glbinfobyte = lpStream[d++]; @@ -214,17 +218,24 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) // Parse channels for (UINT i=0; i dwPos) break; MODCOMMAND cmd = {0,0,0,0,0,0}; BYTE info = lpStream[d++]; - if (info & 0x80) infobyte[i] = lpStream[d++]; + if (info & 0x80) + { + if (d+1 > dwPos) break; + infobyte[i] = lpStream[d++]; + } // Instrument if (info & 0x40) { + if (d+1 > dwPos) break; cmd.instr = lpStream[d++]; } // Note if (info & 0x20) { + if (d+1 > dwPos) break; cmd.note = lpStream[d++]; if ((cmd.note) && (cmd.note < 0xfe)) cmd.note &= 0x7f; if ((cmd.note) && (cmd.note < 128)) cmd.note += 24; @@ -232,12 +243,15 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) // Volume if (info & 0x10) { + if (d+1 > dwPos) break; cmd.volcmd = VOLCMD_VOLUME; cmd.vol = (lpStream[d++]+3)>>2; } // Effect 1 if (info & 0x08) { + if (d+2 > dwPos) break; + BYTE efx = lpStream[d++]; BYTE eval = lpStream[d++]; switch(efx) @@ -259,6 +273,8 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) // Effect 2 if (info & 0x04) { + if (d+2 > dwPos) break; + BYTE efx = lpStream[d++]; BYTE eval = lpStream[d++]; switch(efx) @@ -288,6 +304,8 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) // Effect 3 if (info & 0x02) { + if (d+2 > dwPos) break; + BYTE efx = lpStream[d++]; BYTE eval = lpStream[d++]; switch(efx) @@ -371,22 +389,24 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) #endif if (dwPos + 8 >= dwMemLength) break; } - dwMemPos += patt->patsize + 8; } + dwMemPos += patt->patsize + 8; break; // "SMPI": Sample Info case 0x49504d53: { hasSMPI = 1; - DMFSMPI *pds = (DMFSMPI *)(lpStream+dwMemPos); - if (pds->size <= dwMemLength - dwMemPos) + const DMFSMPI *pds = (DMFSMPI *)(lpStream+dwMemPos); + if ((pds->size >= dwMemLength) || (dwMemPos + 8 > dwMemLength - pds->size)) goto dmfexit; + if (pds->size >= 1) { DWORD dwPos = dwMemPos + 9; m_nSamples = pds->samples; if (m_nSamples >= MAX_SAMPLES) m_nSamples = MAX_SAMPLES-1; for (UINT iSmp=1; iSmp<=m_nSamples; iSmp++) { + if (dwPos >= dwMemPos + pds->size + 8) break; UINT namelen = lpStream[dwPos]; smplflags[iSmp] = 0; if (dwPos+namelen+1+sizeof(DMFSAMPLE) > dwMemPos+pds->size+8) break; @@ -397,7 +417,7 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) m_szNames[iSmp][rlen] = 0; } dwPos += namelen + 1; - DMFSAMPLE *psh = (DMFSAMPLE *)(lpStream+dwPos); + const DMFSAMPLE *psh = (DMFSAMPLE *)(lpStream+dwPos); MODINSTRUMENT *psmp = &Ins[iSmp]; psmp->nLength = psh->len; psmp->nLoopStart = psh->loopstart; @@ -424,7 +444,7 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) { DWORD dwPos = dwMemPos + 8; UINT ismpd = 0; - for (UINT iSmp=1; iSmp<=m_nSamples; iSmp++) + for (UINT iSmp=1; iSmp<=m_nSamples && !hasSMPD; iSmp++) { ismpd++; DWORD pksize; @@ -457,6 +477,7 @@ BOOL CSoundFile::ReadDMF(const BYTE *lpStream, DWORD dwMemLength) } dwPos += pksize; } + hasSMPD = 1; dwMemPos = dwPos; } break; @@ -519,8 +540,8 @@ BYTE DMFReadBits(DMF_HTREE *tree, UINT nbits) { tree->bitnum--; } else - { - tree->bitbuf = (tree->ibuf < tree->ibufmax) ? *(tree->ibuf++) : 0; + if (tree->ibuf < tree->ibufmax) { + tree->bitbuf = *(tree->ibuf++); tree->bitnum = 7; } if (tree->bitbuf & 1) x |= bitv; @@ -575,14 +596,24 @@ int DMFUnpack(LPBYTE psample, LPBYTE ibuf, LPBYTE ibufmax, UINT maxlen) DMF_HTREE tree; UINT actnode; BYTE value, sign, delta = 0; - + memset(&tree, 0, sizeof(tree)); tree.ibuf = ibuf; tree.ibufmax = ibufmax; DMFNewNode(&tree); value = 0; + + if (tree.ibuf >= ibufmax) return tree.ibuf - ibuf; + for (UINT i=0; i= tree.ibufmax) && (!tree.bitnum)) + { + #ifdef DMFLOG + Log("DMFUnpack: unexpected EOF at output byte %d / %d\n", i, maxlen); + #endif + break; + } actnode = 0; sign = DMFReadBits(&tree, 1); do From f42a35440d45bf27512e87807bb22a93391aa7d8 Mon Sep 17 00:00:00 2001 From: AliceLR Date: Tue, 22 Jun 2021 19:24:45 -0600 Subject: [PATCH 4/8] 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. --- src/load_mdl.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/load_mdl.cpp b/src/load_mdl.cpp index 8f2724fc..572eee12 100644 --- a/src/load_mdl.cpp +++ b/src/load_mdl.cpp @@ -217,6 +217,7 @@ BOOL CSoundFile::ReadMDL(const BYTE *lpStream, DWORD dwMemLength) m_szNames[0][31] = 0; norders = pmib->norders; if (norders > MAX_ORDERS) norders = MAX_ORDERS; + if (blocklen < sizeof(MDLINFOBLOCK) + norders - sizeof(pmib->seq)) return FALSE; m_nRestartPos = pmib->repeatpos; m_nDefaultGlobalVolume = pmib->globalvol; m_nDefaultTempo = pmib->tempo; @@ -287,7 +288,7 @@ BOOL CSoundFile::ReadMDL(const BYTE *lpStream, DWORD dwMemLength) case 0x4949: ninstruments = lpStream[dwMemPos]; dwPos = dwMemPos+1; - for (i=0; i= MAX_INSTRUMENTS) || (!nins)) break; @@ -301,6 +302,7 @@ BOOL CSoundFile::ReadMDL(const BYTE *lpStream, DWORD dwMemLength) memcpy(penv->name, lpStream+dwPos+2, 32); penv->nGlobalVol = 64; penv->nPPC = 5*12; + if (34 + 14u*lpStream[dwPos+1] > dwMemLength - dwPos) break; for (j=0; j dwMemLength - dwTrackPos ) len = 0; + if ( len > dwMemLength - (lpTracks - lpStream) ) len = 0; UnpackMDLTrack(m, m_nChannels, PatternSize[ipat], nTrack, lpTracks, len); } From 163eeeccd27b23b0e4754fee6f168699be98f461 Mon Sep 17 00:00:00 2001 From: AliceLR Date: Tue, 22 Jun 2021 19:25:29 -0600 Subject: [PATCH 5/8] 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. --- src/load_mt2.cpp | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/load_mt2.cpp b/src/load_mt2.cpp index 692bf515..a5d15e85 100644 --- a/src/load_mt2.cpp +++ b/src/load_mt2.cpp @@ -211,6 +211,7 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) memcpy(m_szNames[0], pfh->szSongName, 32); m_szNames[0][31] = 0; dwMemPos = sizeof(MT2FILEHEADER); + if (dwMemPos+2 > dwMemLength) return TRUE; nDrumDataLen = *(WORD *)(lpStream + dwMemPos); dwDrumDataPos = dwMemPos + 2; if (nDrumDataLen >= 2) pdd = (MT2DRUMSDATA *)(lpStream+dwDrumDataPos); @@ -236,7 +237,7 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) DWORD dwId = *(DWORD *)(lpStream+dwMemPos); DWORD dwLen = *(DWORD *)(lpStream+dwMemPos+4); dwMemPos += 8; - if (dwMemPos + dwLen > dwMemLength) return TRUE; + if (dwLen >= dwMemLength || dwMemPos > dwMemLength - dwLen) return TRUE; #ifdef MT2DEBUG CHAR s[5]; memcpy(s, &dwId, 4); @@ -274,7 +275,7 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) const MT2PATTERN *pmp = (MT2PATTERN *)(lpStream+dwMemPos); UINT wDataLen = (pmp->wDataLen + 1) & ~1; dwMemPos += 6; - if (dwMemPos + wDataLen > dwMemLength) break; + if (dwMemPos > dwMemLength - wDataLen || wDataLen > dwMemLength) break; UINT nLines = pmp->wLines; if ((iPat < MAX_PATTERNS) && (nLines > 0) && (nLines <= 256)) { @@ -288,7 +289,7 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) UINT len = wDataLen; if (pfh->fulFlags & 1) // Packed Patterns { - BYTE *p = (BYTE *)(lpStream+dwMemPos); + const BYTE *p = lpStream+dwMemPos; UINT pos = 0, row=0, ch=0; while (pos < len) { @@ -297,6 +298,7 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) UINT rptcount = 0; if (infobyte == 0xff) { + if (pos + 2 > len) break; rptcount = p[pos++]; infobyte = p[pos++]; #if 0 @@ -310,13 +312,13 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) { UINT patpos = row*m_nChannels+ch; cmd.note = cmd.instr = cmd.vol = cmd.pan = cmd.fxcmd = cmd.fxparam1 = cmd.fxparam2 = 0; - if (infobyte & 1) cmd.note = p[pos++]; - if (infobyte & 2) cmd.instr = p[pos++]; - if (infobyte & 4) cmd.vol = p[pos++]; - if (infobyte & 8) cmd.pan = p[pos++]; - if (infobyte & 16) cmd.fxcmd = p[pos++]; - if (infobyte & 32) cmd.fxparam1 = p[pos++]; - if (infobyte & 64) cmd.fxparam2 = p[pos++]; + if ((infobyte & 1) && (pos < len)) cmd.note = p[pos++]; + if ((infobyte & 2) && (pos < len)) cmd.instr = p[pos++]; + if ((infobyte & 4) && (pos < len)) cmd.vol = p[pos++]; + if ((infobyte & 8) && (pos < len)) cmd.pan = p[pos++]; + if ((infobyte & 16) && (pos < len)) cmd.fxcmd = p[pos++]; + if ((infobyte & 32) && (pos < len)) cmd.fxparam1 = p[pos++]; + if ((infobyte & 64) && (pos < len)) cmd.fxparam2 = p[pos++]; #ifdef MT2DEBUG if (cmd.fxcmd) { @@ -332,11 +334,12 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) } else { const MT2COMMAND *p = (MT2COMMAND *)(lpStream+dwMemPos); + UINT pos = 0; UINT n = 0; - while ((len > sizeof(MT2COMMAND)) && (n < m_nChannels*nLines)) + while ((pos + sizeof(MT2COMMAND) <= len) && (n < m_nChannels*nLines)) { ConvertMT2Command(this, m, p); - len -= sizeof(MT2COMMAND); + pos += sizeof(MT2COMMAND); n++; p++; m++; @@ -419,8 +422,10 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) #ifdef MT2DEBUG if (iIns <= pfh->wInstruments) Log(" Instrument #%d at offset %04X: %d bytes\n", iIns, dwMemPos, pmi->dwDataLen); #endif - if (((LONG)pmi->dwDataLen > 0) && (dwMemPos <= dwMemLength - 40) && (pmi->dwDataLen <= dwMemLength - (dwMemPos + 40))) + if (pmi->dwDataLen > dwMemLength - (dwMemPos+36)) return TRUE; + if (pmi->dwDataLen > 0) { + if (dwMemPos + sizeof(MT2INSTRUMENT) - 4 > dwMemLength) return TRUE; InstrMap[iIns-1] = pmi; if (penv) { @@ -433,6 +438,7 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) if (pfh->wVersion <= 0x201) { DWORD dwEnvPos = dwMemPos + sizeof(MT2INSTRUMENT) - 4; + if (dwEnvPos + 2*sizeof(MT2ENVELOPE) > dwMemLength) return TRUE; pehdr[0] = (MT2ENVELOPE *)(lpStream+dwEnvPos); pehdr[1] = (MT2ENVELOPE *)(lpStream+dwEnvPos+8); pehdr[2] = pehdr[3] = NULL; @@ -442,10 +448,12 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) } else { DWORD dwEnvPos = dwMemPos + sizeof(MT2INSTRUMENT); + if (dwEnvPos > dwMemLength) return TRUE; for (UINT i=0; i<4; i++) { if (pmi->wEnvFlags1 & (1< dwMemLength) return TRUE; pehdr[i] = (MT2ENVELOPE *)(lpStream+dwEnvPos); pedata[i] = (WORD *)pehdr[i]->EnvData; dwEnvPos += sizeof(MT2ENVELOPE); @@ -540,6 +548,7 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) { memcpy(m_szNames[iSmp], pms->szName, 32); } + if (pms->dwDataLen > dwMemLength - (dwMemPos+36)) return TRUE; if (pms->dwDataLen > 0) { SampleMap[iSmp-1] = pms; @@ -573,12 +582,12 @@ BOOL CSoundFile::ReadMT2(LPCBYTE lpStream, DWORD dwMemLength) #endif for (UINT iMap=0; iMap<255; iMap++) if (InstrMap[iMap]) { - if (dwMemPos+8 > dwMemLength) return TRUE; const MT2INSTRUMENT *pmi = InstrMap[iMap]; INSTRUMENTHEADER *penv = NULL; if (iMapwSamples; iGrp++) { + if (dwMemPos+8 > dwMemLength) return TRUE; if (penv) { const MT2GROUP *pmg = (MT2GROUP *)(lpStream+dwMemPos); From 3d539550fed1fa7d322041016a930c31f3b48497 Mon Sep 17 00:00:00 2001 From: AliceLR Date: Tue, 22 Jun 2021 19:27:09 -0600 Subject: [PATCH 6/8] 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. --- src/load_psm.cpp | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/src/load_psm.cpp b/src/load_psm.cpp index 4e72afad..f3713e25 100644 --- a/src/load_psm.cpp +++ b/src/load_psm.cpp @@ -98,7 +98,7 @@ void swap_PSMSAMPLE(PSMSAMPLE* p){ BOOL CSoundFile::ReadPSM(LPCBYTE lpStream, DWORD dwMemLength) //----------------------------------------------------------- { - PSMCHUNK pfh = *(const PSMCHUNK *)lpStream; + PSMCHUNK pfh; DWORD dwMemPos, dwSongPos; DWORD smpnames[MAX_SAMPLES]; DWORD patptrs[MAX_PATTERNS]; @@ -108,6 +108,7 @@ BOOL CSoundFile::ReadPSM(LPCBYTE lpStream, DWORD dwMemLength) if (dwMemLength < 256) return FALSE; // Swap chunk + pfh = *(const PSMCHUNK *)lpStream; swap_PSMCHUNK(&pfh); // Chunk0: "PSM ",filesize,"FILE" @@ -279,14 +280,17 @@ BOOL CSoundFile::ReadPSM(LPCBYTE lpStream, DWORD dwMemLength) { PSMPATTERN pPsmPat = *(const PSMPATTERN *)(lpStream+patptrs[nPat]+8); swap_PSMPATTERN(&pPsmPat); - ULONG len = *(DWORD *)(lpStream+patptrs[nPat]+4) - 12; + PSMCHUNK pchunk = *(const PSMCHUNK *)(lpStream+patptrs[nPat]); + swap_PSMCHUNK(&pchunk); + + ULONG len = pchunk.len - 12; UINT nRows = pPsmPat.rows; if (len > pPsmPat.size) len = pPsmPat.size; if ((nRows < 64) || (nRows > 256)) nRows = 64; PatternSize[nPat] = nRows; if ((Patterns[nPat] = AllocatePattern(nRows, m_nChannels)) == NULL) break; MODCOMMAND *m = Patterns[nPat]; - BYTE *p = pPsmPat.data; + const BYTE *p = lpStream + patptrs[nPat] + 20; MODCOMMAND *sp, dummy; UINT pos = 0; UINT row = 0; @@ -313,17 +317,17 @@ BOOL CSoundFile::ReadPSM(LPCBYTE lpStream, DWORD dwMemLength) ch = p[pos++]; if (ch >= m_nChannels) { sp = &dummy; - } else { + } else { sp = &m[ch]; - } + } // Note + Instr - if ((flags & 0x80) && (pos+1 < len)) - { - UINT note = p[pos++]; - note = (note>>4)*12+(note&0x0f)+12+1; - if (note > 0x80) note = 0; + if ((flags & 0x80) && (pos+1 < len)) + { + UINT note = p[pos++]; + note = (note>>4)*12+(note&0x0f)+12+1; + if (note > 0x80) note = 0; sp->note = note; - } + } if ((flags & 0x40) && (pos+1 < len)) { UINT nins = p[pos++]; @@ -331,7 +335,8 @@ BOOL CSoundFile::ReadPSM(LPCBYTE lpStream, DWORD dwMemLength) //if (!nPat) Log("note+ins: %02X.%02X\n", note, nins); if ((!nPat) && (nins >= m_nSamples)) Log("WARNING: invalid instrument number (%d)\n", nins); #endif - sp->instr = samplemap[nins]; + if (nins < MAX_SAMPLES) + sp->instr = samplemap[nins]; } // Volume if ((flags & 0x20) && (pos < len)) From 84d9cbac356c29e2778c2aca649ac85f8d320ad6 Mon Sep 17 00:00:00 2001 From: AliceLR Date: Tue, 22 Jun 2021 19:28:20 -0600 Subject: [PATCH 7/8] Fix MMCMP depacker bugs found by libFuzzer. * MMCMP: fix out-of-bounds reads due to broken initial subblock bounds check. * MMCMP: reduce slow loads and potential bugs caused by broken and redundant subblock packing bounds checks. --- src/mmcmp.cpp | 39 +++++++++++++++++---------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/src/mmcmp.cpp b/src/mmcmp.cpp index 890df2f5..6f3a4a13 100644 --- a/src/mmcmp.cpp +++ b/src/mmcmp.cpp @@ -185,15 +185,20 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) LPMMCMPSUBBLOCK psubblk; if (dwMemPos >= dwMemLength - 20) break; - memcpy(tmp1, lpMemFile+dwMemPos, 28); pblk = (LPMMCMPBLOCK)(tmp1); psubblk = (LPMMCMPSUBBLOCK)(tmp1+20); + + memcpy(pblk, lpMemFile+dwMemPos, 20); swap_block(pblk); - swap_subblock(psubblk); - if (dwMemPos + 20 + pblk->sub_blk*8 >= dwMemLength) break; + if (pblk->sub_blk*8 >= dwMemLength - dwMemPos - 20) break; dwSubPos = dwMemPos + 20; dwMemPos += 20 + pblk->sub_blk*8; + + if (!pblk->sub_blk) continue; + memcpy(psubblk, lpMemFile + dwSubPos, 8); + swap_subblock(psubblk); + #ifdef MMCMP_LOG Log("block %d: flags=%04X sub_blocks=%d", nBlock, (UINT)pblk->flags, (UINT)pblk->sub_blk); Log(" pksize=%d unpksize=%d", pblk->pk_size, pblk->unpk_size); @@ -213,7 +218,7 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) #endif memcpy(pBuffer+psubblk->unpk_pos, lpMemFile+dwMemPos, psubblk->unpk_size); dwMemPos += psubblk->unpk_size; - memcpy(tmp1+20,lpMemFile+dwSubPos+i*8,8); + memcpy(psubblk,lpMemFile+dwSubPos+i*8,8); swap_subblock(psubblk); } } else @@ -227,8 +232,8 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) UINT numbits = pblk->num_bits; UINT subblk = 0, oldval = 0; - if (dwSize * 2 > dwFileSize-psubblk->unpk_pos || - psubblk->unpk_pos > dwMemLength-dwMemPos) + if (psubblk->unpk_pos >= dwFileSize || + dwSize * 2 > dwFileSize - psubblk->unpk_pos) break; #ifdef MMCMP_LOG @@ -248,11 +253,6 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) UINT newval = 0x10000; DWORD d = bb.GetBits(numbits+1); - if ((psubblk->unpk_pos >= dwFileSize) || - (psubblk->unpk_size >= dwFileSize) || - (psubblk->unpk_size > dwFileSize - psubblk->unpk_pos)) - dwPos = dwSize; - if (d >= MMCMP16BitCommands[numbits]) { UINT nFetch = MMCMP16BitFetch[numbits]; @@ -293,12 +293,12 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) if (dwPos >= dwSize) { subblk++; - memcpy(tmp1+20,lpMemFile+dwSubPos+subblk*8,8); + memcpy(psubblk,lpMemFile+dwSubPos+subblk*8,8); swap_subblock(psubblk); dwPos = 0; dwSize = psubblk->unpk_size >> 1; if ( psubblk->unpk_pos >= dwFileSize || - dwSize * 2 > dwFileSize ) { + dwSize * 2 > dwFileSize - psubblk->unpk_pos) { break; } pDest = (LPWORD)(pBuffer + psubblk->unpk_pos); @@ -315,8 +315,8 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) UINT subblk = 0, oldval = 0; LPCBYTE ptable = lpMemFile+dwMemPos; - if (dwSize > dwFileSize-psubblk->unpk_pos || - psubblk->unpk_pos > dwMemLength-dwMemPos) + if (psubblk->unpk_pos >= dwFileSize || + dwSize > dwFileSize - psubblk->unpk_pos) break; bb.bitcount = 0; @@ -330,11 +330,6 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) UINT newval = 0x100; DWORD d = bb.GetBits(numbits+1); - if ((psubblk->unpk_pos >= dwFileSize) || - (psubblk->unpk_size >= dwFileSize) || - (psubblk->unpk_size > dwFileSize - (psubblk->unpk_pos))) - dwPos = dwSize; - if (d >= MMCMP8BitCommands[numbits]) { UINT nFetch = MMCMP8BitFetch[numbits]; @@ -370,12 +365,12 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength) if (dwPos >= dwSize) { subblk++; - memcpy(tmp1+20,lpMemFile+dwSubPos+subblk*8,8); + memcpy(psubblk,lpMemFile+dwSubPos+subblk*8,8); swap_subblock(psubblk); dwPos = 0; dwSize = psubblk->unpk_size; if ( psubblk->unpk_pos >= dwFileSize || - dwSize > dwFileSize ) + dwSize > dwFileSize - psubblk->unpk_pos) break; pDest = pBuffer + psubblk->unpk_pos; } From c647d2c9cf8bf0b868345a335051eecbdd079d3f Mon Sep 17 00:00:00 2001 From: AliceLR Date: Tue, 22 Jun 2021 19:29:01 -0600 Subject: [PATCH 8/8] Fix MIDI and PAT loader bugs found by libFuzzer. * MIDI: fix stack corruption caused by using sprintf on an undersized buffer. * MIDI: fix out-of-bounds reads caused by no bounds checks being performed on the mmread* and mid_read* functions. * MIDI: fix NULL dereferences when attempting to reuse tracks containing no events. * MIDI: fix leaks and hangs caused by not freeing MIDI structs or clearing the reentry flag when m_nChannels is 0. * MIDI: fix leaks caused by not freeing MIDI structs when PAT_Load_Instruments fails. * MIDI: fix leaks caused by not freeing MIDTRACKs. * PAT: fix out-of-bounds reads caused by invalid GM patches. --- src/load_mid.cpp | 97 ++++++++++++++++++++++++++++++++---------------- src/load_pat.cpp | 2 +- 2 files changed, 66 insertions(+), 33 deletions(-) diff --git a/src/load_mid.cpp b/src/load_mid.cpp index 5992fa64..e8b518d8 100644 --- a/src/load_mid.cpp +++ b/src/load_mid.cpp @@ -105,6 +105,7 @@ typedef struct { char *mm; unsigned int sz; int pos; + int err; } MMFILE; static void mmfseek(MMFILE *mmfile, long p, int whence) @@ -130,21 +131,38 @@ static long mmftell(MMFILE *mmfile) static BYTE mmreadUBYTE(MMFILE *mmfile) { BYTE b; + if ((unsigned int)mmfile->pos >= mmfile->sz) + { + mmfile->err = EOF; + return 0; + } b = (BYTE)mmfile->mm[mmfile->pos]; mmfile->pos++; return b; } -static void mmreadUBYTES(BYTE *buf, long sz, MMFILE *mmfile) +static unsigned long mmreadUBYTES(BYTE *buf, unsigned long sz, MMFILE *mmfile) { + if ((unsigned long)mmfile->pos + sz >= mmfile->sz) + { + mmfile->err = EOF; + return 0; + } memcpy(buf, &mmfile->mm[mmfile->pos], sz); mmfile->pos += sz; + return sz; } -static void mmreadSBYTES(char *buf, long sz, MMFILE *mmfile) +static unsigned long mmreadSBYTES(char *buf, long sz, MMFILE *mmfile) { + if ((unsigned long)mmfile->pos + sz >= mmfile->sz) + { + mmfile->err = EOF; + return 0; + } memcpy(buf, &mmfile->mm[mmfile->pos], sz); mmfile->pos += sz; + return sz; } /**********************************************************************/ @@ -417,6 +435,10 @@ static MIDTRACK *mid_locate_track(MIDHANDLE *h, int mch, int pos) for( tr=h->track; tr; tr=tr->next ) { if( tr->chan == mch ) { e = tr->workevent; + if (!e) { + trunused = tr; + break; + } if( h->tracktime > e->tracktick + tmin ) { tmin = h->tracktime - e->tracktick; trunused = tr; @@ -433,6 +455,10 @@ static MIDTRACK *mid_locate_track(MIDHANDLE *h, int mch, int pos) for( tr=h->track; tr; tr=tr->next ) { if( tr->chan == mch ) { e = tr->workevent; + if (!e) { + trunused = tr; + break; + } if( h->tracktime >= e->tracktick + tmin ) { tmin = h->tracktime - e->tracktick; trunused = tr; @@ -446,6 +472,10 @@ static MIDTRACK *mid_locate_track(MIDHANDLE *h, int mch, int pos) tmin = 0; for( tr=h->track; tr; tr=tr->next ) { e = tr->workevent; + if (!e) { + trunused = tr; + break; + } if( h->tracktime >= e->tracktick + tmin ) { tmin = h->tracktime - e->tracktick; trunused = tr; @@ -709,14 +739,15 @@ static void mid_add_pitchwheel(MIDHANDLE *h, int mch, int wheel) static uint32_t mid_read_long(MIDHANDLE *h) { BYTE buf[4]; - mmreadUBYTES(buf, 4, h->mmf); + if (!mmreadUBYTES(buf, 4, h->mmf)) return -1; + return (buf[0]<<24)|(buf[1]<<16)|(buf[2]<<8)|buf[3]; } static short int mid_read_short(MIDHANDLE *h) { BYTE buf[2]; - mmreadUBYTES(buf, 2, h->mmf); + if (!mmreadUBYTES(buf, 2, h->mmf)) return -1; return (buf[0]<<8)|buf[1]; } @@ -750,6 +781,7 @@ BOOL CSoundFile::TestMID(const BYTE *lpStream, DWORD dwMemLength) MMFILE mm; mm.mm = (char *)lpStream; mm.sz = dwMemLength; + mm.err = 0; h.mmf = &mm; if (h.mmf->sz < 4) return FALSE; mmfseek(h.mmf,0,SEEK_SET); @@ -791,6 +823,7 @@ static void MID_CleanupTracks(MIDHANDLE *handle) for( tp=handle->track; tp; tp = tn ) { tn=tp->next; MID_CleanupTrack(tp); + free(tp); } handle->track = NULL; } @@ -1074,7 +1107,7 @@ static void mid_notes_to_percussion(MIDTRACK *tp, ULONG adjust, ULONG tmin) } } if( ton > toff ) { - char info[32]; + char info[64]; sprintf(info,"%ld > %ld note %d", (long)ton, (long)toff, n); mid_message("drum track ends with note on (%s)", info); } @@ -1128,7 +1161,7 @@ static void mid_prog_to_notes(MIDTRACK *tp, ULONG adjust, ULONG tmin) } } if( ton > toff ) { - char info[40]; + char info[128]; sprintf(info,"channel %d, %ld > %ld note %d", tp->chan + 1, (long)ton, (long)toff, n); mid_message("melody track ends with note on (%s)", info); } @@ -1172,19 +1205,14 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) BYTE *p; while( avoid_reentry ) sleep(1); avoid_reentry = 1; - if( !TestMID(lpStream, dwMemLength) ) { - avoid_reentry = 0; - return FALSE; - } + if( !TestMID(lpStream, dwMemLength) ) goto ErrorExit; h = MID_Init(); - if( !h ) { - avoid_reentry = 0; - return FALSE; - } + if( !h ) goto ErrorExit; h->mmf = &mm; mm.mm = (char *)lpStream; mm.sz = dwMemLength; mm.pos = 0; + mm.err = 0; h->debug = getenv(ENV_MMMID_DEBUG); h->verbose = getenv(ENV_MMMID_VERBOSE); pat_resetsmp(); @@ -1193,6 +1221,8 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) h->midiformat = mid_read_short(h); h->miditracks = mid_read_short(h); h->resolution = mid_read_short(h); + if (mm.err) goto ErrorCleanup; + // at this point the h->mmf is positioned at first miditrack if( h->midiformat == 0 ) h->miditracks = 1; if( h->resolution & 0x8000 ) @@ -1205,11 +1235,8 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) m_nDefaultTempo = 0; h->tracktime = 0; h->speed = 6; - if (h->miditracks == 0) { - MID_Cleanup(h); - avoid_reentry = 0; - return FALSE; - } + if (h->miditracks == 0) goto ErrorCleanup; + p = (BYTE *)getenv(ENV_MMMID_SPEED); if( p && isdigit(*p) && p[0] != '0' && p[1] == '\0' ) { // transform speed @@ -1247,19 +1274,18 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) } for( t=0; t<(uint32_t)h->miditracks; t++ ) { if( h->verbose ) printf("Parsing track %d\n", t+1); - mmreadSBYTES(buf,4,h->mmf); + if (!mmreadSBYTES(buf,4,h->mmf)) goto ErrorCleanup; buf[4] = '\0'; if( strcmp(buf,"MTrk") ) { mid_message("invalid track-chunk '%s' is not 'MTrk'",buf); - MID_Cleanup(h); - avoid_reentry = 0; - return FALSE; + goto ErrorCleanup; } miditracklen = mid_read_long(h); - if (mm.sz < miditracklen) continue; + if (mm.err || mm.sz < miditracklen) continue; runningstatus = 0; if( t && h->midiformat == 1 ) mid_rewind_tracks(h); // tracks sound simultaneously while( miditracklen > 0 ) { + if (mm.err) break; miditracklen -= mid_read_delta(h); midibyte[0] = mid_read_byte(h); miditracklen--; @@ -1378,6 +1404,7 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) t, (long)(h->tracktime), midibyte[0]); while( midibyte[0] != 0xf7 ) { midibyte[0] = mid_read_byte(h); + if (mm.err) break; miditracklen--; if( h->debug ) printf(" %02X", midibyte[0]); } @@ -1399,6 +1426,7 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) t, (long)(h->tracktime), metalen); while( metalen > 0 ) { midibyte[1] = mid_read_byte(h); + if (mm.err) break; metalen--; miditracklen--; if( h->debug ) printf(" %02X", midibyte[1]); @@ -1411,13 +1439,14 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) metalen = h->deltatime; if( metalen > 31 ) metalen = 31; if( metalen ) { - mmreadSBYTES(buf, metalen, h->mmf); + if (!mmreadSBYTES(buf, metalen, h->mmf)) break; miditracklen -= metalen; } buf[metalen] = '\0'; metalen = h->deltatime - metalen; while( metalen > 0 ) { midibyte[1] = mid_read_byte(h); + if (mm.err) break; metalen--; miditracklen--; } @@ -1467,7 +1496,7 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) } if( miditracklen < 1 && (runningstatus != 0xff || midibyte[0] != 0x2f) ) { delta = mmftell(h->mmf); - mmreadSBYTES(buf,4,h->mmf); + if (!mmreadSBYTES(buf,4,h->mmf)) break; buf[4] = '\0'; if( strcmp(buf,"MTrk") ) { miditracklen = 0x7fffffff; @@ -1545,15 +1574,13 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) m_dwSongFlags = SONG_LINEARSLIDES; m_nMinPeriod = 28 << 2; m_nMaxPeriod = 1712 << 3; - if (m_nChannels == 0) - return FALSE; + if (m_nChannels == 0) goto ErrorCleanup; + // orderlist for(t=0; t < numpats; t++) Order[t] = t; - if( !PAT_Load_Instruments(this) ) { - avoid_reentry = 0; - return FALSE; - } + if( !PAT_Load_Instruments(this) ) goto ErrorCleanup; + // ============================== // Load the pattern info now! if( MID_ReadPatterns(Patterns, PatternSize, h, numpats, m_nChannels) ) { @@ -1581,4 +1608,10 @@ BOOL CSoundFile::ReadMID(const BYTE *lpStream, DWORD dwMemLength) MID_Cleanup(h); // we dont need it anymore avoid_reentry = 0; // it is safe now, I'm finished return TRUE; + +ErrorCleanup: + MID_Cleanup(h); +ErrorExit: + avoid_reentry = 0; + return FALSE; } diff --git a/src/load_pat.cpp b/src/load_pat.cpp index adcccc3f..6ea86b40 100644 --- a/src/load_pat.cpp +++ b/src/load_pat.cpp @@ -188,7 +188,7 @@ int pat_numinstr(void) int pat_smptogm(int smp) { - if( smp < MAXSMP ) + if( smp && smp < MAXSMP && pat_gm_used[smp - 1] < MAXSMP ) return pat_gm_used[smp - 1]; return 1; }