Skip to content

Commit 0431c20

Browse files
KonstantyKonstanty
authored
Merge pull request #73 from AliceLR/cleanup-66
Clean up and fix redundant/missing checks in OKT, S3M, ULT loaders.
2 parents 0b71303 + 9401191 commit 0431c20

File tree

3 files changed

+51
-27
lines changed

3 files changed

+51
-27
lines changed

‎src/load_okt.cpp‎

Lines changed: 49 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,20 @@
1313

1414
//#pragma warning(disable:4244)
1515

16+
#define MAGIC(a,b,c,d) (((a) << 24UL) | ((b) << 16UL) | ((c) << 8UL) | (d))
17+
18+
#pragma pack(1)
1619
typedef struct OKTFILEHEADER
1720
{
1821
DWORD okta; // "OKTA"
1922
DWORD song; // "SONG"
2023
DWORD cmod; // "CMOD"
21-
DWORD fixed8;
24+
DWORD cmodlen;
2225
BYTE chnsetup[8];
2326
DWORD samp; // "SAMP"
2427
DWORD samplen;
2528
} OKTFILEHEADER;
2629

27-
2830
typedef struct OKTSAMPLE
2931
{
3032
CHAR name[20];
@@ -36,20 +38,29 @@ typedef struct OKTSAMPLE
3638
BYTE pad2;
3739
BYTE pad3;
3840
} OKTSAMPLE;
41+
#pragma pack()
42+
3943

44+
static DWORD readBE32(const BYTE *v)
45+
{
46+
return (v[0] << 24UL) | (v[1] << 16UL) | (v[2] << 8UL) | v[3];
47+
}
4048

4149
BOOL CSoundFile::ReadOKT(const BYTE *lpStream, DWORD dwMemLength)
4250
//---------------------------------------------------------------
4351
{
4452
const OKTFILEHEADER *pfh = (OKTFILEHEADER *)lpStream;
45-
DWORD dwMemPos = sizeof(OKTFILEHEADER);
53+
DWORD dwMemPos = sizeof(OKTFILEHEADER), dwSize;
4654
UINT nsamples = 0, npatterns = 0, norders = 0;
4755

4856
if ((!lpStream) || (dwMemLength < 1024)) return FALSE;
49-
if ((pfh->okta != 0x41544B4F) || (pfh->song != 0x474E4F53)
50-
|| (pfh->cmod != 0x444F4D43) || (pfh->chnsetup[0]) || (pfh->chnsetup[2])
51-
|| (pfh->chnsetup[4]) || (pfh->chnsetup[6]) || (pfh->fixed8 != 0x08000000)
52-
|| (pfh->samp != 0x504D4153)) return FALSE;
57+
if ((bswapBE32(pfh->okta) != MAGIC('O','K','T','A'))
58+
|| (bswapBE32(pfh->song) != MAGIC('S','O','N','G'))
59+
|| (bswapBE32(pfh->cmod) != MAGIC('C','M','O','D'))
60+
|| (bswapBE32(pfh->cmodlen) != 8)
61+
|| (pfh->chnsetup[0]) || (pfh->chnsetup[2])
62+
|| (pfh->chnsetup[4]) || (pfh->chnsetup[6])
63+
|| (bswapBE32(pfh->samp) != MAGIC('S','A','M','P'))) return FALSE;
5364
m_nType = MOD_TYPE_OKT;
5465
m_nChannels = 4 + pfh->chnsetup[1] + pfh->chnsetup[3] + pfh->chnsetup[5] + pfh->chnsetup[7];
5566
if (m_nChannels > MAX_CHANNELS) m_nChannels = MAX_CHANNELS;
@@ -62,7 +73,7 @@ BOOL CSoundFile::ReadOKT(const BYTE *lpStream, DWORD dwMemLength)
6273
if (dwMemPos >= dwMemLength - sizeof(OKTSAMPLE)) return TRUE;
6374
if (smp < MAX_SAMPLES)
6475
{
65-
OKTSAMPLE *psmp = (OKTSAMPLE *)(lpStream + dwMemPos);
76+
const OKTSAMPLE *psmp = (const OKTSAMPLE *)(lpStream + dwMemPos);
6677
MODINSTRUMENT *pins = &Ins[smp];
6778

6879
memcpy(m_szNames[smp], psmp->name, 20);
@@ -79,41 +90,51 @@ BOOL CSoundFile::ReadOKT(const BYTE *lpStream, DWORD dwMemLength)
7990
}
8091
// SPEE
8192
if (dwMemPos >= dwMemLength - 12) return TRUE;
82-
if (*((DWORD *)(lpStream + dwMemPos)) == 0x45455053)
93+
if (readBE32(lpStream + dwMemPos) == MAGIC('S','P','E','E'))
8394
{
8495
m_nDefaultSpeed = lpStream[dwMemPos+9];
85-
dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8;
96+
97+
dwSize = readBE32(lpStream + dwMemPos + 4);
98+
if (dwSize > dwMemLength - 8 || dwMemPos > dwMemLength - dwSize - 8) return TRUE;
99+
dwMemPos += dwSize + 8;
86100
}
87101
// SLEN
88102
if (dwMemPos + 10 > dwMemLength) return TRUE;
89-
if (*((DWORD *)(lpStream + dwMemPos)) == 0x4E454C53)
103+
if (readBE32(lpStream + dwMemPos) == MAGIC('S','L','E','N'))
90104
{
91-
if (dwMemPos + 10 > dwMemLength) return TRUE;
92105
npatterns = lpStream[dwMemPos+9];
93-
dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8;
106+
107+
dwSize = readBE32(lpStream + dwMemPos + 4);
108+
if (dwSize > dwMemLength - 8 || dwMemPos > dwMemLength - dwSize - 8) return TRUE;
109+
dwMemPos += dwSize + 8;
94110
}
95111
// PLEN
96112
if (dwMemPos + 10 > dwMemLength) return TRUE;
97-
if (*((DWORD *)(lpStream + dwMemPos)) == 0x4E454C50)
113+
if (readBE32(lpStream + dwMemPos) == MAGIC('P','L','E','N'))
98114
{
99-
if (dwMemPos + 10 > dwMemLength) return TRUE;
100115
norders = lpStream[dwMemPos+9];
101-
dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8;
116+
117+
dwSize = readBE32(lpStream + dwMemPos + 4);
118+
if (dwSize > dwMemLength - 8 || dwMemPos > dwMemLength - dwSize - 8) return TRUE;
119+
dwMemPos += dwSize + 8;
102120
}
103121
// PATT
104122
if (dwMemPos + 8 > dwMemLength) return TRUE;
105-
if (*((DWORD *)(lpStream + dwMemPos)) == 0x54544150)
123+
if (readBE32(lpStream + dwMemPos) == MAGIC('P','A','T','T'))
106124
{
107125
UINT orderlen = norders;
108126
if (orderlen >= MAX_ORDERS) orderlen = MAX_ORDERS-1;
109127
if (dwMemPos + 8 + orderlen > dwMemLength) return TRUE;
110128
for (UINT i=0; i<orderlen; i++) Order[i] = lpStream[dwMemPos+8+i];
111129
for (UINT j=orderlen; j>1; j--) { if (Order[j-1]) break; Order[j-1] = 0xFF; }
112-
dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8;
130+
131+
dwSize = readBE32(lpStream + dwMemPos + 4);
132+
if (dwSize > dwMemLength - 8 || dwMemPos > dwMemLength - dwSize - 8) return TRUE;
133+
dwMemPos += dwSize + 8;
113134
}
114135
// PBOD
115136
UINT npat = 0;
116-
while ((dwMemPos < dwMemLength - 10) && (*((DWORD *)(lpStream + dwMemPos)) == 0x444F4250))
137+
while ((dwMemPos < dwMemLength - 10) && (readBE32(lpStream + dwMemPos) == MAGIC('P','B','O','D')))
117138
{
118139
DWORD dwPos = dwMemPos + 10;
119140
UINT rows = lpStream[dwMemPos+9];
@@ -185,15 +206,21 @@ BOOL CSoundFile::ReadOKT(const BYTE *lpStream, DWORD dwMemLength)
185206
}
186207
}
187208
npat++;
188-
dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8;
209+
210+
dwSize = readBE32(lpStream + dwMemPos + 4);
211+
if (dwSize > dwMemLength - 8 || dwMemPos > dwMemLength - dwSize - 8) return TRUE;
212+
dwMemPos += dwSize + 8;
189213
}
190214
// SBOD
191215
UINT nsmp = 1;
192-
while ((dwMemPos < dwMemLength-10) && (*((DWORD *)(lpStream + dwMemPos)) == 0x444F4253))
216+
while ((dwMemPos < dwMemLength-10) && (readBE32(lpStream + dwMemPos) == MAGIC('S','B','O','D')))
193217
{
194218
if (nsmp < MAX_SAMPLES) ReadSample(&Ins[nsmp], RS_PCM8S, (LPSTR)(lpStream+dwMemPos+8), dwMemLength-dwMemPos-8);
195-
dwMemPos += bswapBE32(*((DWORD *)(lpStream + dwMemPos + 4))) + 8;
196219
nsmp++;
220+
221+
dwSize = readBE32(lpStream + dwMemPos + 4);
222+
if (dwSize > dwMemLength - 8 || dwMemPos > dwMemLength - dwSize - 8) return TRUE;
223+
dwMemPos += dwSize + 8;
197224
}
198225
return TRUE;
199226
}

‎src/load_s3m.cpp‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,6 @@ BOOL CSoundFile::ReadS3M(const BYTE *lpStream, DWORD dwMemLength)
282282
}
283283
if (psfh.panning_present == 252)
284284
{
285-
if (dwMemPos + 32 > dwMemLength) return FALSE;
286-
287285
const BYTE *chnpan = lpStream+dwMemPos;
288286
if (dwMemPos > dwMemLength - 32) return FALSE;
289287
for (UINT i=0; i<32; i++) if (chnpan[i] & 0x20)

‎src/load_ult.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,16 +155,15 @@ BOOL CSoundFile::ReadUlt(const BYTE *lpStream, DWORD dwMemLength)
155155
UINT row = 0;
156156
while (row < 64)
157157
{
158-
if (dwMemPos + 5 > dwMemLength) return TRUE;
158+
if (dwMemPos > dwMemLength - 5) return TRUE;
159159
UINT rep = 1;
160160
UINT note = lpStream[dwMemPos++];
161161
if (note == 0xFC)
162162
{
163-
if (dwMemPos + 7 > dwMemLength) return TRUE;
164163
rep = lpStream[dwMemPos];
165164
note = lpStream[dwMemPos+1];
166165
dwMemPos += 2;
167-
if (dwMemPos + 4 > dwMemLength) return TRUE;
166+
if (dwMemPos > dwMemLength - 4) return TRUE;
168167
}
169168

170169
UINT instr = lpStream[dwMemPos++];

0 commit comments

Comments
 (0)