Skip to content

Commit 13e1d79

Browse files
AliceLRsezero
authored andcommitted
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. Konstanty#58
1 parent 019e926 commit 13e1d79

File tree

1 file changed

+17
-22
lines changed

1 file changed

+17
-22
lines changed

src/mmcmp.cpp

Lines changed: 17 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -186,15 +186,20 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
186186
LPMMCMPSUBBLOCK psubblk;
187187

188188
if (dwMemPos >= dwMemLength - 20) break;
189-
memcpy(tmp1, lpMemFile+dwMemPos, 28);
190189
pblk = (LPMMCMPBLOCK)(tmp1);
191190
psubblk = (LPMMCMPSUBBLOCK)(tmp1+20);
191+
192+
memcpy(pblk, lpMemFile+dwMemPos, 20);
192193
swap_block(pblk);
193-
swap_subblock(psubblk);
194194

195-
if (dwMemPos + 20 + pblk->sub_blk*8 >= dwMemLength) break;
195+
if (pblk->sub_blk*8 >= dwMemLength - dwMemPos - 20) break;
196196
dwSubPos = dwMemPos + 20;
197197
dwMemPos += 20 + pblk->sub_blk*8;
198+
199+
if (!pblk->sub_blk) continue;
200+
memcpy(psubblk, lpMemFile + dwSubPos, 8);
201+
swap_subblock(psubblk);
202+
198203
#ifdef MMCMP_LOG
199204
Log("block %d: flags=%04X sub_blocks=%d", nBlock, (UINT)pblk->flags, (UINT)pblk->sub_blk);
200205
Log(" pksize=%d unpksize=%d", pblk->pk_size, pblk->unpk_size);
@@ -214,7 +219,7 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
214219
#endif
215220
memcpy(pBuffer+psubblk->unpk_pos, lpMemFile+dwMemPos, psubblk->unpk_size);
216221
dwMemPos += psubblk->unpk_size;
217-
memcpy(tmp1+20,lpMemFile+dwSubPos+i*8,8);
222+
memcpy(psubblk,lpMemFile+dwSubPos+i*8,8);
218223
swap_subblock(psubblk);
219224
}
220225
} else
@@ -228,8 +233,8 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
228233
UINT numbits = pblk->num_bits;
229234
UINT subblk = 0, oldval = 0;
230235

231-
if (dwSize * 2 > dwFileSize-psubblk->unpk_pos ||
232-
psubblk->unpk_pos > dwMemLength-dwMemPos)
236+
if (psubblk->unpk_pos >= dwFileSize ||
237+
dwSize * 2 > dwFileSize - psubblk->unpk_pos)
233238
break;
234239

235240
#ifdef MMCMP_LOG
@@ -249,11 +254,6 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
249254
UINT newval = 0x10000;
250255
DWORD d = bb.GetBits(numbits+1);
251256

252-
if ((psubblk->unpk_pos >= dwFileSize) ||
253-
(psubblk->unpk_size >= dwFileSize) ||
254-
(psubblk->unpk_size > dwFileSize - psubblk->unpk_pos))
255-
dwPos = dwSize;
256-
257257
if (d >= MMCMP16BitCommands[numbits])
258258
{
259259
UINT nFetch = MMCMP16BitFetch[numbits];
@@ -294,12 +294,12 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
294294
if (dwPos >= dwSize)
295295
{
296296
subblk++;
297-
memcpy(tmp1+20,lpMemFile+dwSubPos+subblk*8,8);
297+
memcpy(psubblk,lpMemFile+dwSubPos+subblk*8,8);
298298
swap_subblock(psubblk);
299299
dwPos = 0;
300300
dwSize = psubblk->unpk_size >> 1;
301301
if ( psubblk->unpk_pos >= dwFileSize ||
302-
dwSize * 2 > dwFileSize ) {
302+
dwSize * 2 > dwFileSize - psubblk->unpk_pos) {
303303
break;
304304
}
305305
pDest = (LPWORD)(pBuffer + psubblk->unpk_pos);
@@ -316,8 +316,8 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
316316
UINT subblk = 0, oldval = 0;
317317
LPCBYTE ptable = lpMemFile+dwMemPos;
318318

319-
if (dwSize > dwFileSize-psubblk->unpk_pos ||
320-
psubblk->unpk_pos > dwMemLength-dwMemPos)
319+
if (psubblk->unpk_pos >= dwFileSize ||
320+
dwSize > dwFileSize - psubblk->unpk_pos)
321321
break;
322322

323323
bb.bitcount = 0;
@@ -331,11 +331,6 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
331331
UINT newval = 0x100;
332332
DWORD d = bb.GetBits(numbits+1);
333333

334-
if ((psubblk->unpk_pos >= dwFileSize) ||
335-
(psubblk->unpk_size >= dwFileSize) ||
336-
(psubblk->unpk_size > dwFileSize - (psubblk->unpk_pos)))
337-
dwPos = dwSize;
338-
339334
if (d >= MMCMP8BitCommands[numbits])
340335
{
341336
UINT nFetch = MMCMP8BitFetch[numbits];
@@ -371,12 +366,12 @@ BOOL MMCMP_Unpack(LPCBYTE *ppMemFile, LPDWORD pdwMemLength)
371366
if (dwPos >= dwSize)
372367
{
373368
subblk++;
374-
memcpy(tmp1+20,lpMemFile+dwSubPos+subblk*8,8);
369+
memcpy(psubblk,lpMemFile+dwSubPos+subblk*8,8);
375370
swap_subblock(psubblk);
376371
dwPos = 0;
377372
dwSize = psubblk->unpk_size;
378373
if ( psubblk->unpk_pos >= dwFileSize ||
379-
dwSize > dwFileSize )
374+
dwSize > dwFileSize - psubblk->unpk_pos)
380375
break;
381376
pDest = pBuffer + psubblk->unpk_pos;
382377
}

0 commit comments

Comments
 (0)