From 86b6888f3ece894d02ef625ff16939a14670ed98 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 14:14:28 +0200 Subject: Fixed heap overrun in https://github.com/ladislav-zezula/StormLib/issues/327 --- test/StormTest.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 3276c59..718c910 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -4183,9 +4183,10 @@ static const LPCSTR Test_CreateMpq_Localized[] = // Main #define TEST_COMMAND_LINE -#define TEST_LOCAL_LISTFILE -#define TEST_STREAM_OPERATIONS -#define TEST_MASTER_MIRROR +//#define TEST_LOCAL_LISTFILE +//#define TEST_STREAM_OPERATIONS +//#define TEST_MASTER_MIRROR +#define TEST_OPEN_MPQ #define TEST_OPEN_MPQ #define TEST_REOPEN_MPQ #define TEST_VERIFY_SIGNATURE @@ -4211,7 +4212,7 @@ int _tmain(int argc, TCHAR * argv[]) #ifdef TEST_COMMAND_LINE // Test-open MPQs from the command line. They must be plain name // and must be placed in the Test-MPQs folder - for(int i = 1; i < argc; i++) + for(int i = 2; i < argc; i++) { TestOpenArchive(argv[i], NULL, NULL, 0, &LfBliz); } -- cgit v1.2.3 From c0d7708350d0e38ee71802f14dd34a1dd9732b31 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 16:41:40 +0200 Subject: Fixed too-big-alloc in https://github.com/ladislav-zezula/StormLib/issues/329 --- src/SBaseFileTable.cpp | 10 ++++++++++ test/StormTest.cpp | 8 ++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/src/SBaseFileTable.cpp b/src/SBaseFileTable.cpp index 274129d..84cfe45 100644 --- a/src/SBaseFileTable.cpp +++ b/src/SBaseFileTable.cpp @@ -635,6 +635,8 @@ DWORD ConvertMpqHeaderToFormat4( // Size of the hi-block table if(pHeader->HiBlockTablePos64) { + if(pHeader->HiBlockTablePos64 > FileSize) + return ERROR_FILE_CORRUPT; pHeader->HiBlockTableSize64 = MaxOffset - pHeader->HiBlockTablePos64; MaxOffset = pHeader->HiBlockTablePos64; } @@ -642,6 +644,8 @@ DWORD ConvertMpqHeaderToFormat4( // Size of the block table if(BlockTablePos64) { + if(BlockTablePos64 > FileSize) + return ERROR_FILE_CORRUPT; pHeader->BlockTableSize64 = MaxOffset - BlockTablePos64; MaxOffset = BlockTablePos64; } @@ -649,6 +653,8 @@ DWORD ConvertMpqHeaderToFormat4( // Size of the hash table if(HashTablePos64) { + if(HashTablePos64 > FileSize) + return ERROR_FILE_CORRUPT; pHeader->HashTableSize64 = MaxOffset - HashTablePos64; MaxOffset = HashTablePos64; } @@ -656,6 +662,8 @@ DWORD ConvertMpqHeaderToFormat4( // Size of the BET table if(pHeader->BetTablePos64) { + if(pHeader->BetTablePos64 > FileSize) + return ERROR_FILE_CORRUPT; pHeader->BetTableSize64 = MaxOffset - pHeader->BetTablePos64; MaxOffset = pHeader->BetTablePos64; } @@ -663,6 +671,8 @@ DWORD ConvertMpqHeaderToFormat4( // Size of the HET table if(pHeader->HetTablePos64) { + if(pHeader->HetTablePos64 > FileSize) + return ERROR_FILE_CORRUPT; pHeader->HetTableSize64 = MaxOffset - pHeader->HetTablePos64; // MaxOffset = pHeader->HetTablePos64; } diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 718c910..00c5e13 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -4002,6 +4002,11 @@ static const TEST_INFO1 TestList_MasterMirror[] = static const TEST_INFO1 Test_OpenMpqs[] = { + // PoC's by Gabe Sherman from FuturesLab + {_T("pocs/MPQ_2024_01_HeapOverrun.mpq"), NULL, "7008f95dcbc4e5d840830c176dec6969", 14}, + {_T("pocs/MPQ_2024_02_StackOverflow.mpq"), NULL, "7093fcbcc9674b3e152e74e8e8a937bb", 4}, + {_T("pocs/MPQ_2024_03_TooBigAlloc.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + // Correct or damaged archives {_T("MPQ_1997_v1_Diablo1_DIABDAT.MPQ"), NULL, "554b538541e42170ed41cb236483489e", 2910, &TwoFilesD1}, // Base MPQ from Diablo 1 {_T("MPQ_1997_v1_patch_rt_SC1B.mpq"), NULL, "43fe7d362955be68a708486e399576a7", 10}, // From Starcraft 1 BETA @@ -4182,12 +4187,11 @@ static const LPCSTR Test_CreateMpq_Localized[] = //----------------------------------------------------------------------------- // Main -#define TEST_COMMAND_LINE +//#define TEST_COMMAND_LINE //#define TEST_LOCAL_LISTFILE //#define TEST_STREAM_OPERATIONS //#define TEST_MASTER_MIRROR #define TEST_OPEN_MPQ -#define TEST_OPEN_MPQ #define TEST_REOPEN_MPQ #define TEST_VERIFY_SIGNATURE #define TEST_REPLACE_FILE -- cgit v1.2.3 From 34a29066eaf9050ba88fd02cd8a370ac0fc7c7ea Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 17:07:29 +0200 Subject: Fixed heap overflow in https://github.com/ladislav-zezula/StormLib/issues/331 --- src/SBaseFileTable.cpp | 123 ++++++++++++++++++++++++++++--------------------- test/StormTest.cpp | 4 +- 2 files changed, 73 insertions(+), 54 deletions(-) (limited to 'test') diff --git a/src/SBaseFileTable.cpp b/src/SBaseFileTable.cpp index 84cfe45..5589320 100644 --- a/src/SBaseFileTable.cpp +++ b/src/SBaseFileTable.cpp @@ -1669,6 +1669,19 @@ void FreeHetTable(TMPQHetTable * pHetTable) //----------------------------------------------------------------------------- // Support for BET table +static bool VerifyBetHeaderSize(TMPQArchive * ha, TMPQBetHeader * pBetHeader) +{ + LPBYTE pbSrcData = (LPBYTE)(pBetHeader + 1); + LPBYTE pbSrcEnd = (LPBYTE)(pBetHeader) + pBetHeader->dwTableSize; + + // Move past the flags + pbSrcData = pbSrcData + (pBetHeader->dwFlagCount * sizeof(DWORD)) + (pBetHeader->dwEntryCount * pBetHeader->dwTableEntrySize); + if(pbSrcData > pbSrcEnd) + return false; + + return true; +} + static void CreateBetHeader( TMPQArchive * ha, TMPQBetHeader * pBetHeader) @@ -1802,68 +1815,72 @@ static TMPQBetTable * TranslateBetTable( // Note that if it's not, it is not a problem //assert(pBetHeader->dwEntryCount == ha->pHetTable->dwEntryCount); - // Create translated table - pBetTable = CreateBetTable(pBetHeader->dwEntryCount); - if(pBetTable != NULL) + // Verify an obviously-wrong values + if(VerifyBetHeaderSize(ha, pBetHeader)) { - // Copy the variables from the header to the BetTable - pBetTable->dwTableEntrySize = pBetHeader->dwTableEntrySize; - pBetTable->dwBitIndex_FilePos = pBetHeader->dwBitIndex_FilePos; - pBetTable->dwBitIndex_FileSize = pBetHeader->dwBitIndex_FileSize; - pBetTable->dwBitIndex_CmpSize = pBetHeader->dwBitIndex_CmpSize; - pBetTable->dwBitIndex_FlagIndex = pBetHeader->dwBitIndex_FlagIndex; - pBetTable->dwBitIndex_Unknown = pBetHeader->dwBitIndex_Unknown; - pBetTable->dwBitCount_FilePos = pBetHeader->dwBitCount_FilePos; - pBetTable->dwBitCount_FileSize = pBetHeader->dwBitCount_FileSize; - pBetTable->dwBitCount_CmpSize = pBetHeader->dwBitCount_CmpSize; - pBetTable->dwBitCount_FlagIndex = pBetHeader->dwBitCount_FlagIndex; - pBetTable->dwBitCount_Unknown = pBetHeader->dwBitCount_Unknown; - - // Since we don't know what the "unknown" is, we'll assert when it's zero - assert(pBetTable->dwBitCount_Unknown == 0); - - // Allocate array for flags - if(pBetHeader->dwFlagCount != 0) + // Create translated table + pBetTable = CreateBetTable(pBetHeader->dwEntryCount); + if(pBetTable != NULL) { - // Allocate array for file flags and load it - pBetTable->pFileFlags = STORM_ALLOC(DWORD, pBetHeader->dwFlagCount); - if(pBetTable->pFileFlags != NULL) + // Copy the variables from the header to the BetTable + pBetTable->dwTableEntrySize = pBetHeader->dwTableEntrySize; + pBetTable->dwBitIndex_FilePos = pBetHeader->dwBitIndex_FilePos; + pBetTable->dwBitIndex_FileSize = pBetHeader->dwBitIndex_FileSize; + pBetTable->dwBitIndex_CmpSize = pBetHeader->dwBitIndex_CmpSize; + pBetTable->dwBitIndex_FlagIndex = pBetHeader->dwBitIndex_FlagIndex; + pBetTable->dwBitIndex_Unknown = pBetHeader->dwBitIndex_Unknown; + pBetTable->dwBitCount_FilePos = pBetHeader->dwBitCount_FilePos; + pBetTable->dwBitCount_FileSize = pBetHeader->dwBitCount_FileSize; + pBetTable->dwBitCount_CmpSize = pBetHeader->dwBitCount_CmpSize; + pBetTable->dwBitCount_FlagIndex = pBetHeader->dwBitCount_FlagIndex; + pBetTable->dwBitCount_Unknown = pBetHeader->dwBitCount_Unknown; + + // Since we don't know what the "unknown" is, we'll assert when it's zero + assert(pBetTable->dwBitCount_Unknown == 0); + + // Allocate array for flags + if(pBetHeader->dwFlagCount != 0) { - LengthInBytes = pBetHeader->dwFlagCount * sizeof(DWORD); - memcpy(pBetTable->pFileFlags, pbSrcData, LengthInBytes); - BSWAP_ARRAY32_UNSIGNED(pBetTable->pFileFlags, LengthInBytes); - pbSrcData += LengthInBytes; + // Allocate array for file flags and load it + pBetTable->pFileFlags = STORM_ALLOC(DWORD, pBetHeader->dwFlagCount); + if(pBetTable->pFileFlags != NULL) + { + LengthInBytes = pBetHeader->dwFlagCount * sizeof(DWORD); + memcpy(pBetTable->pFileFlags, pbSrcData, LengthInBytes); + BSWAP_ARRAY32_UNSIGNED(pBetTable->pFileFlags, LengthInBytes); + pbSrcData += LengthInBytes; + } + + // Save the number of flags + pBetTable->dwFlagCount = pBetHeader->dwFlagCount; } - // Save the number of flags - pBetTable->dwFlagCount = pBetHeader->dwFlagCount; - } + // Load the bit-based file table + pBetTable->pFileTable = TMPQBits::Create(pBetTable->dwTableEntrySize * pBetHeader->dwEntryCount, 0); + if(pBetTable->pFileTable != NULL) + { + LengthInBytes = (pBetTable->pFileTable->NumberOfBits + 7) / 8; + memcpy(pBetTable->pFileTable->Elements, pbSrcData, LengthInBytes); + pbSrcData += LengthInBytes; + } - // Load the bit-based file table - pBetTable->pFileTable = TMPQBits::Create(pBetTable->dwTableEntrySize * pBetHeader->dwEntryCount, 0); - if(pBetTable->pFileTable != NULL) - { - LengthInBytes = (pBetTable->pFileTable->NumberOfBits + 7) / 8; - memcpy(pBetTable->pFileTable->Elements, pbSrcData, LengthInBytes); - pbSrcData += LengthInBytes; - } + // Fill the sizes of BET hash + pBetTable->dwBitTotal_NameHash2 = pBetHeader->dwBitTotal_NameHash2; + pBetTable->dwBitExtra_NameHash2 = pBetHeader->dwBitExtra_NameHash2; + pBetTable->dwBitCount_NameHash2 = pBetHeader->dwBitCount_NameHash2; - // Fill the sizes of BET hash - pBetTable->dwBitTotal_NameHash2 = pBetHeader->dwBitTotal_NameHash2; - pBetTable->dwBitExtra_NameHash2 = pBetHeader->dwBitExtra_NameHash2; - pBetTable->dwBitCount_NameHash2 = pBetHeader->dwBitCount_NameHash2; + // Create and load the array of BET hashes + pBetTable->pNameHashes = TMPQBits::Create(pBetTable->dwBitTotal_NameHash2 * pBetHeader->dwEntryCount, 0); + if(pBetTable->pNameHashes != NULL) + { + LengthInBytes = (pBetTable->pNameHashes->NumberOfBits + 7) / 8; + memcpy(pBetTable->pNameHashes->Elements, pbSrcData, LengthInBytes); + // pbSrcData += LengthInBytes; + } - // Create and load the array of BET hashes - pBetTable->pNameHashes = TMPQBits::Create(pBetTable->dwBitTotal_NameHash2 * pBetHeader->dwEntryCount, 0); - if(pBetTable->pNameHashes != NULL) - { - LengthInBytes = (pBetTable->pNameHashes->NumberOfBits + 7) / 8; - memcpy(pBetTable->pNameHashes->Elements, pbSrcData, LengthInBytes); -// pbSrcData += LengthInBytes; + // Dump both tables +// DumpHetAndBetTable(ha->pHetTable, pBetTable); } - - // Dump both tables -// DumpHetAndBetTable(ha->pHetTable, pBetTable); } } } diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 00c5e13..e8c9bd5 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -4006,6 +4006,8 @@ static const TEST_INFO1 Test_OpenMpqs[] = {_T("pocs/MPQ_2024_01_HeapOverrun.mpq"), NULL, "7008f95dcbc4e5d840830c176dec6969", 14}, {_T("pocs/MPQ_2024_02_StackOverflow.mpq"), NULL, "7093fcbcc9674b3e152e74e8e8a937bb", 4}, {_T("pocs/MPQ_2024_03_TooBigAlloc.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_04_HeapOverflow.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_05_HeapOverflow.mpq"), NULL, "0539ae020719654a0ea6e2627a8195f8", 14}, // Correct or damaged archives {_T("MPQ_1997_v1_Diablo1_DIABDAT.MPQ"), NULL, "554b538541e42170ed41cb236483489e", 2910, &TwoFilesD1}, // Base MPQ from Diablo 1 @@ -4187,7 +4189,7 @@ static const LPCSTR Test_CreateMpq_Localized[] = //----------------------------------------------------------------------------- // Main -//#define TEST_COMMAND_LINE +#define TEST_COMMAND_LINE //#define TEST_LOCAL_LISTFILE //#define TEST_STREAM_OPERATIONS //#define TEST_MASTER_MIRROR -- cgit v1.2.3 From 3643858d00d26165404837a1f0f7640a84873c30 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 18:22:59 +0200 Subject: Fixed reading big files from non-corrupt MPQs --- src/SBaseFileTable.cpp | 22 ++++++++++++++-------- test/StormTest.cpp | 11 ++++------- 2 files changed, 18 insertions(+), 15 deletions(-) (limited to 'test') diff --git a/src/SBaseFileTable.cpp b/src/SBaseFileTable.cpp index 5589320..8f5c7b3 100644 --- a/src/SBaseFileTable.cpp +++ b/src/SBaseFileTable.cpp @@ -617,6 +617,9 @@ DWORD ConvertMpqHeaderToFormat4( pHeader->BetTablePos64 = 0; } + // Fixup malformed MPQ header sizes + pHeader->dwHeaderSize = STORMLIB_MIN(pHeader->dwHeaderSize, MPQ_HEADER_SIZE_V3); + // // We need to calculate the compressed size of each table. We assume the following order: // 1) HET table @@ -685,7 +688,6 @@ DWORD ConvertMpqHeaderToFormat4( // Verify header MD5. Header MD5 is calculated from the MPQ header since the 'MPQ\x1A' // signature until the position of header MD5 at offset 0xC0 - // Apparently, Starcraft II only accepts MPQ headers where the MPQ header hash matches // If MD5 doesn't match, we ignore this offset. We also ignore it if there's no MD5 at all if(!IsValidMD5(pHeader->MD5_MpqHeader)) @@ -696,6 +698,9 @@ DWORD ConvertMpqHeaderToFormat4( // Byteswap after header MD5 is verified BSWAP_TMPQHEADER(pHeader, MPQ_FORMAT_VERSION_4); + // Fixup malformed MPQ header sizes + pHeader->dwHeaderSize = MPQ_HEADER_SIZE_V4; + // HiBlockTable must be 0 for archives under 4GB if((pHeader->ArchiveSize64 >> 0x20) == 0 && pHeader->HiBlockTablePos64 != 0) return ERROR_FAKE_MPQ_HEADER; @@ -784,9 +789,12 @@ static bool IsValidHashEntry1(TMPQArchive * ha, TMPQHash * pHash, TMPQBlock * pB pBlock = pBlockTable + MPQ_BLOCK_INDEX(pHash); // Check whether this is an existing file - // Also we do not allow to be file size greater than 2GB - if((pBlock->dwFlags & MPQ_FILE_EXISTS) && (pBlock->dwFSize & 0x80000000) == 0) + if(pBlock->dwFlags & MPQ_FILE_EXISTS) { + // We don't allow to be file size greater than 2GB in malformed archives + if((ha->dwFlags & MPQ_FLAG_MALFORMED) && (pBlock->dwFSize >= 0x80000000)) + return false; + // The begin of the file must be within the archive ByteOffset = FileOffsetFromMpqOffset(ha, pBlock->dwFilePos); return (ByteOffset < ha->FileSize); @@ -1669,17 +1677,14 @@ void FreeHetTable(TMPQHetTable * pHetTable) //----------------------------------------------------------------------------- // Support for BET table -static bool VerifyBetHeaderSize(TMPQArchive * ha, TMPQBetHeader * pBetHeader) +static bool VerifyBetHeaderSize(TMPQArchive * /* ha */, TMPQBetHeader * pBetHeader) { LPBYTE pbSrcData = (LPBYTE)(pBetHeader + 1); LPBYTE pbSrcEnd = (LPBYTE)(pBetHeader) + pBetHeader->dwTableSize; // Move past the flags pbSrcData = pbSrcData + (pBetHeader->dwFlagCount * sizeof(DWORD)) + (pBetHeader->dwEntryCount * pBetHeader->dwTableEntrySize); - if(pbSrcData > pbSrcEnd) - return false; - - return true; + return (pbSrcData <= pbSrcEnd); } static void CreateBetHeader( @@ -3156,6 +3161,7 @@ DWORD SaveMPQTables(TMPQArchive * ha) CalculateDataBlockHash(pHeader, MPQ_HEADER_SIZE_V4 - MD5_DIGEST_SIZE, pHeader->MD5_MpqHeader); // Write the MPQ header to the file + assert(pHeader->dwHeaderSize <= sizeof(SaveMpqHeader)); memcpy(&SaveMpqHeader, pHeader, pHeader->dwHeaderSize); BSWAP_TMPQHEADER(&SaveMpqHeader, MPQ_FORMAT_VERSION_1); BSWAP_TMPQHEADER(&SaveMpqHeader, MPQ_FORMAT_VERSION_2); diff --git a/test/StormTest.cpp b/test/StormTest.cpp index e8c9bd5..540557a 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3787,16 +3787,13 @@ static DWORD TestReplaceFile(LPCTSTR szMpqPlainName, LPCTSTR szFilePlainName, LP static void Test_PlayingSpace() { - HANDLE hFile = NULL; + SFILE_FIND_DATA sf; HANDLE hMpq = NULL; - if(SFileOpenArchive(_T("(4)Duskwood.w3m"), 0, 0, &hMpq)) + if(SFileOpenArchive(_T("e:\\poc11"), 0, 0, &hMpq)) { - if(SFileOpenFileEx(hMpq, "war3map.j", 0, &hFile)) - { - SFileSetFileLocale(hFile, 1033); - SFileCloseFile(hFile); - } + SFileFindFirstFile(hMpq, "*", &sf, NULL); + SFileAddWave(hMpq, _T("e:\\Ladik\\Incoming\\poc11"), "poc11", MPQ_FILE_FIX_KEY, 1); SFileCloseArchive(hMpq); } } -- cgit v1.2.3 From 355665c0ab214cae667681858cc1d8c3b0a41d4a Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 18:40:56 +0200 Subject: Fixed kernelmode heap overflow (via ReadFile), described in https://github.com/ladislav-zezula/StormLib/issues/333 --- src/SFileReadFile.cpp | 7 ++++++- test/StormTest.cpp | 13 ++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) (limited to 'test') diff --git a/src/SFileReadFile.cpp b/src/SFileReadFile.cpp index 8ab5f48..180d428 100644 --- a/src/SFileReadFile.cpp +++ b/src/SFileReadFile.cpp @@ -261,6 +261,8 @@ static DWORD ReadMpqFileSingleUnit(TMPQFile * hf, void * pvBuffer, DWORD dwFileP // If the file sector is not loaded yet, do it if(hf->dwSectorOffs != 0) { + DWORD cbRawData = hf->dwDataSize; + // Is the file compressed? if(pFileEntry->dwFlags & MPQ_FILE_COMPRESS_MASK) { @@ -268,11 +270,14 @@ static DWORD ReadMpqFileSingleUnit(TMPQFile * hf, void * pvBuffer, DWORD dwFileP pbCompressed = STORM_ALLOC(BYTE, pFileEntry->dwCmpSize); if(pbCompressed == NULL) return ERROR_NOT_ENOUGH_MEMORY; + + // Redirect reading pbRawData = pbCompressed; + cbRawData = pFileEntry->dwCmpSize; } // Load the raw (compressed, encrypted) data - if(!FileStream_Read(ha->pStream, &RawFilePos, pbRawData, pFileEntry->dwCmpSize)) + if(!FileStream_Read(ha->pStream, &RawFilePos, pbRawData, cbRawData)) { STORM_FREE(pbCompressed); return GetLastError(); diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 540557a..eeb0a24 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3787,6 +3787,7 @@ static DWORD TestReplaceFile(LPCTSTR szMpqPlainName, LPCTSTR szFilePlainName, LP static void Test_PlayingSpace() { +/* SFILE_FIND_DATA sf; HANDLE hMpq = NULL; @@ -3796,6 +3797,7 @@ static void Test_PlayingSpace() SFileAddWave(hMpq, _T("e:\\Ladik\\Incoming\\poc11"), "poc11", MPQ_FILE_FIX_KEY, 1); SFileCloseArchive(hMpq); } +*/ } //----------------------------------------------------------------------------- @@ -4000,11 +4002,12 @@ static const TEST_INFO1 TestList_MasterMirror[] = static const TEST_INFO1 Test_OpenMpqs[] = { // PoC's by Gabe Sherman from FuturesLab - {_T("pocs/MPQ_2024_01_HeapOverrun.mpq"), NULL, "7008f95dcbc4e5d840830c176dec6969", 14}, - {_T("pocs/MPQ_2024_02_StackOverflow.mpq"), NULL, "7093fcbcc9674b3e152e74e8e8a937bb", 4}, - {_T("pocs/MPQ_2024_03_TooBigAlloc.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, - {_T("pocs/MPQ_2024_04_HeapOverflow.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, - {_T("pocs/MPQ_2024_05_HeapOverflow.mpq"), NULL, "0539ae020719654a0ea6e2627a8195f8", 14}, + //{_T("pocs/MPQ_2024_01_HeapOverrun.mpq"), NULL, "7008f95dcbc4e5d840830c176dec6969", 14}, + //{_T("pocs/MPQ_2024_02_StackOverflow.mpq"), NULL, "7093fcbcc9674b3e152e74e8e8a937bb", 4}, + //{_T("pocs/MPQ_2024_03_TooBigAlloc.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + //{_T("pocs/MPQ_2024_04_HeapOverflow.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + //{_T("pocs/MPQ_2024_05_HeapOverflow.mpq"), NULL, "0539ae020719654a0ea6e2627a8195f8", 14}, + {_T("pocs/MPQ_2024_06_HeapOverflowReadFile.mpq"), NULL, "d41d8cd98f00b204e9800998ecf8427e", 1}, // Correct or damaged archives {_T("MPQ_1997_v1_Diablo1_DIABDAT.MPQ"), NULL, "554b538541e42170ed41cb236483489e", 2910, &TwoFilesD1}, // Base MPQ from Diablo 1 -- cgit v1.2.3 From dccc1068b0246697b70af2d7b6f8f7418d623324 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 18:45:43 +0200 Subject: Fixed division by zero in https://github.com/ladislav-zezula/StormLib/issues/334 --- src/FileStream.cpp | 2 +- test/StormTest.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/src/FileStream.cpp b/src/FileStream.cpp index bc5618e..b66098c 100644 --- a/src/FileStream.cpp +++ b/src/FileStream.cpp @@ -1198,7 +1198,7 @@ static bool FlatStream_LoadBitmap(TBlockStream * pStream) BSWAP_ARRAY32_UNSIGNED((LPDWORD)(&Footer), sizeof(FILE_BITMAP_FOOTER)); // Verify if there is actually a footer - if(Footer.Signature == ID_FILE_BITMAP_FOOTER && Footer.Version == 0x03) + if(Footer.Signature == ID_FILE_BITMAP_FOOTER && Footer.Version == 0x03 && Footer.BlockSize != 0) { // Get the offset of the bitmap, number of blocks and size of the bitmap ByteOffset = MAKE_OFFSET64(Footer.MapOffsetHi, Footer.MapOffsetLo); diff --git a/test/StormTest.cpp b/test/StormTest.cpp index eeb0a24..84ed667 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -4008,6 +4008,7 @@ static const TEST_INFO1 Test_OpenMpqs[] = //{_T("pocs/MPQ_2024_04_HeapOverflow.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, //{_T("pocs/MPQ_2024_05_HeapOverflow.mpq"), NULL, "0539ae020719654a0ea6e2627a8195f8", 14}, {_T("pocs/MPQ_2024_06_HeapOverflowReadFile.mpq"), NULL, "d41d8cd98f00b204e9800998ecf8427e", 1}, + {_T("pocs/MPQ_2024_07_InvalidBitmapFooter.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, // Correct or damaged archives {_T("MPQ_1997_v1_Diablo1_DIABDAT.MPQ"), NULL, "554b538541e42170ed41cb236483489e", 2910, &TwoFilesD1}, // Base MPQ from Diablo 1 -- cgit v1.2.3 From f70bfc0eb6eaf09fe653d55b977efcbb25bf4a00 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 18:56:43 +0200 Subject: Fixed division by zero in https://github.com/ladislav-zezula/StormLib/issues/335 --- src/SFileOpenArchive.cpp | 10 +++++++--- test/StormTest.cpp | 8 ++++---- 2 files changed, 11 insertions(+), 7 deletions(-) (limited to 'test') diff --git a/src/SFileOpenArchive.cpp b/src/SFileOpenArchive.cpp index a97ecea..587aa96 100644 --- a/src/SFileOpenArchive.cpp +++ b/src/SFileOpenArchive.cpp @@ -501,10 +501,14 @@ bool WINAPI SFileOpenArchive( break; } - // Set the size of file sector - ha->dwSectorSize = (0x200 << ha->pHeader->wSectorSize); + // Set the size of file sector. Be sure to check for integer overflow + if((ha->dwSectorSize = (0x200 << ha->pHeader->wSectorSize)) == 0) + dwErrCode = ERROR_FILE_CORRUPT; + } - // Verify if any of the tables doesn't start beyond the end of the file + // Verify if any of the tables doesn't start beyond the end of the file + if(dwErrCode == ERROR_SUCCESS) + { dwErrCode = VerifyMpqTablePositions(ha, FileSize); } diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 84ed667..290c419 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3788,13 +3788,11 @@ static DWORD TestReplaceFile(LPCTSTR szMpqPlainName, LPCTSTR szFilePlainName, LP static void Test_PlayingSpace() { /* - SFILE_FIND_DATA sf; HANDLE hMpq = NULL; - if(SFileOpenArchive(_T("e:\\poc11"), 0, 0, &hMpq)) + if(SFileOpenArchive(_T("e:\\poc17"), 0, 0, &hMpq)) { - SFileFindFirstFile(hMpq, "*", &sf, NULL); - SFileAddWave(hMpq, _T("e:\\Ladik\\Incoming\\poc11"), "poc11", MPQ_FILE_FIX_KEY, 1); + SFileCompactArchive(hMpq, _T("e:\\Ladik\\Incoming\\poc17"), true); SFileCloseArchive(hMpq); } */ @@ -4009,6 +4007,8 @@ static const TEST_INFO1 Test_OpenMpqs[] = //{_T("pocs/MPQ_2024_05_HeapOverflow.mpq"), NULL, "0539ae020719654a0ea6e2627a8195f8", 14}, {_T("pocs/MPQ_2024_06_HeapOverflowReadFile.mpq"), NULL, "d41d8cd98f00b204e9800998ecf8427e", 1}, {_T("pocs/MPQ_2024_07_InvalidBitmapFooter.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_08_InvalidSectorSize.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_09_InvalidSectorSize.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, // Correct or damaged archives {_T("MPQ_1997_v1_Diablo1_DIABDAT.MPQ"), NULL, "554b538541e42170ed41cb236483489e", 2910, &TwoFilesD1}, // Base MPQ from Diablo 1 -- cgit v1.2.3 From b35dc1e4d9d1a70c77a7554df87900486388fe0b Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 19:59:39 +0200 Subject: Regression tests for the first half of the bugs found by Gabe Sherman --- src/SBaseFileTable.cpp | 2 +- test/StormTest.cpp | 17 +++++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'test') diff --git a/src/SBaseFileTable.cpp b/src/SBaseFileTable.cpp index 8f5c7b3..fc9418a 100644 --- a/src/SBaseFileTable.cpp +++ b/src/SBaseFileTable.cpp @@ -1683,7 +1683,7 @@ static bool VerifyBetHeaderSize(TMPQArchive * /* ha */, TMPQBetHeader * pBetHead LPBYTE pbSrcEnd = (LPBYTE)(pBetHeader) + pBetHeader->dwTableSize; // Move past the flags - pbSrcData = pbSrcData + (pBetHeader->dwFlagCount * sizeof(DWORD)) + (pBetHeader->dwEntryCount * pBetHeader->dwTableEntrySize); + pbSrcData = pbSrcData + (pBetHeader->dwFlagCount * sizeof(DWORD)) + (pBetHeader->dwEntryCount * pBetHeader->dwTableEntrySize) / 8; return (pbSrcData <= pbSrcEnd); } diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 290c419..31e9981 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3999,12 +3999,13 @@ static const TEST_INFO1 TestList_MasterMirror[] = static const TEST_INFO1 Test_OpenMpqs[] = { + // PoC's by Gabe Sherman from FuturesLab - //{_T("pocs/MPQ_2024_01_HeapOverrun.mpq"), NULL, "7008f95dcbc4e5d840830c176dec6969", 14}, - //{_T("pocs/MPQ_2024_02_StackOverflow.mpq"), NULL, "7093fcbcc9674b3e152e74e8e8a937bb", 4}, - //{_T("pocs/MPQ_2024_03_TooBigAlloc.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, - //{_T("pocs/MPQ_2024_04_HeapOverflow.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, - //{_T("pocs/MPQ_2024_05_HeapOverflow.mpq"), NULL, "0539ae020719654a0ea6e2627a8195f8", 14}, + {_T("pocs/MPQ_2024_01_HeapOverrun.mpq"), NULL, "7008f95dcbc4e5d840830c176dec6969", 14}, + {_T("pocs/MPQ_2024_02_StackOverflow.mpq"), NULL, "7093fcbcc9674b3e152e74e8e8a937bb", 4}, + {_T("pocs/MPQ_2024_03_TooBigAlloc.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_04_HeapOverflow.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_05_HeapOverflow.mpq"), NULL, "0539ae020719654a0ea6e2627a8195f8", 14}, {_T("pocs/MPQ_2024_06_HeapOverflowReadFile.mpq"), NULL, "d41d8cd98f00b204e9800998ecf8427e", 1}, {_T("pocs/MPQ_2024_07_InvalidBitmapFooter.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, {_T("pocs/MPQ_2024_08_InvalidSectorSize.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, @@ -4191,9 +4192,9 @@ static const LPCSTR Test_CreateMpq_Localized[] = // Main #define TEST_COMMAND_LINE -//#define TEST_LOCAL_LISTFILE -//#define TEST_STREAM_OPERATIONS -//#define TEST_MASTER_MIRROR +#define TEST_LOCAL_LISTFILE +#define TEST_STREAM_OPERATIONS +#define TEST_MASTER_MIRROR #define TEST_OPEN_MPQ #define TEST_REOPEN_MPQ #define TEST_VERIFY_SIGNATURE -- cgit v1.2.3 From a26f04c11dd86e949e649a8c0a01eeaeae268c26 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 20:15:04 +0200 Subject: Added buffer overflow checks to the Huffmann decompression (https://github.com/ladislav-zezula/StormLib/issues/336) --- src/huffman/huff.cpp | 50 +++++++++++++++++++++++++++++++++++--------------- src/huffman/huff.h | 2 +- test/StormTest.cpp | 6 ++---- 3 files changed, 38 insertions(+), 20 deletions(-) (limited to 'test') diff --git a/src/huffman/huff.cpp b/src/huffman/huff.cpp index 1b81017..bbb8118 100644 --- a/src/huffman/huff.cpp +++ b/src/huffman/huff.cpp @@ -21,6 +21,11 @@ #include "huff.h" +//----------------------------------------------------------------------------- +// Local defined + +#define HUFF_DECOMPRESS_ERROR 0x1FF + //----------------------------------------------------------------------------- // Table of byte-to-weight values @@ -270,46 +275,50 @@ TInputStream::TInputStream(void * pvInBuffer, size_t cbInBuffer) } // Gets one bit from input stream -unsigned int TInputStream::Get1Bit() +bool TInputStream::Get1Bit(unsigned int & BitValue) { - unsigned int OneBit = 0; - // Ensure that the input stream is reloaded, if there are no bits left if(BitCount == 0) { + // Buffer overflow check + if(pbInBuffer >= pbInBufferEnd) + return false; + // Refill the bit buffer BitBuffer = *pbInBuffer++; BitCount = 8; } // Copy the bit from bit buffer to the variable - OneBit = (BitBuffer & 0x01); + BitValue = (BitBuffer & 0x01); BitBuffer >>= 1; BitCount--; - - return OneBit; + return true; } // Gets the whole byte from the input stream. -unsigned int TInputStream::Get8Bits() +bool TInputStream::Get8Bits(unsigned int & ByteValue) { unsigned int dwReloadByte = 0; - unsigned int dwOneByte = 0; // If there is not enough bits to get the value, // we have to add 8 more bits from the input buffer if(BitCount < 8) { + // Buffer overflow check + if(pbInBuffer >= pbInBufferEnd) + return false; + dwReloadByte = *pbInBuffer++; BitBuffer |= dwReloadByte << BitCount; BitCount += 8; } // Return the lowest 8 its - dwOneByte = (BitBuffer & 0xFF); + ByteValue = (BitBuffer & 0xFF); BitBuffer >>= 8; BitCount -= 8; - return dwOneByte; + return true; } // Gets 7 bits from the stream. DOES NOT remove the bits from input stream @@ -344,6 +353,10 @@ void TInputStream::SkipBits(unsigned int dwBitsToSkip) // we have to add 8 more bits from the input buffer if(BitCount < dwBitsToSkip) { + // Buffer overflow check + if(pbInBuffer >= pbInBufferEnd) + return; + dwReloadByte = *pbInBuffer++; BitBuffer |= dwReloadByte << BitCount; BitCount += 8; @@ -726,7 +739,7 @@ unsigned int THuffmannTree::DecodeOneByte(TInputStream * is) { // Just a sanity check if(pFirst == LIST_HEAD()) - return 0x1FF; + return HUFF_DECOMPRESS_ERROR; // We don't have the quick-link item, we need to parse the tree from its root pItem = pFirst; @@ -735,9 +748,14 @@ unsigned int THuffmannTree::DecodeOneByte(TInputStream * is) // Step down the tree until we find a terminal item while(pItem->pChildLo != NULL) { + unsigned int BitValue = 0; + // If the next bit in the compressed stream is set, we get the higher-weight // child. Otherwise, get the lower-weight child. - pItem = is->Get1Bit() ? pItem->pChildLo->pPrev : pItem->pChildLo; + if(!is->Get1Bit(BitValue)) + return HUFF_DECOMPRESS_ERROR; + + pItem = BitValue ? pItem->pChildLo->pPrev : pItem->pChildLo; BitCount++; // If the number of loaded bits reached 7, @@ -852,7 +870,8 @@ unsigned int THuffmannTree::Decompress(void * pvOutBuffer, unsigned int cbOutLen return 0; // Get the compression type from the input stream - CompressionType = is->Get8Bits(); + if(!is->Get8Bits(CompressionType)) + return 0; bIsCmp0 = (CompressionType == 0) ? 1 : 0; // Build the Huffman tree @@ -863,14 +882,15 @@ unsigned int THuffmannTree::Decompress(void * pvOutBuffer, unsigned int cbOutLen while((DecompressedValue = DecodeOneByte(is)) != 0x100) { // Did an error occur? - if(DecompressedValue == 0x1FF) // An error occurred + if(DecompressedValue == HUFF_DECOMPRESS_ERROR) return 0; // Huffman tree needs to be modified if(DecompressedValue == 0x101) { // The decompressed byte is stored in the next 8 bits - DecompressedValue = is->Get8Bits(); + if(!is->Get8Bits(DecompressedValue)) + return 0; if(!InsertNewBranchAndRebalance(pLast->DecompressedValue, DecompressedValue)) return 0; diff --git a/src/huffman/huff.h b/src/huffman/huff.h index cf1ca4c..e6f9e42 100644 --- a/src/huffman/huff.h +++ b/src/huffman/huff.h @@ -28,7 +28,7 @@ class TInputStream public: TInputStream(void * pvInBuffer, size_t cbInBuffer); - unsigned int Get1Bit(); + bool Get1Bit(unsigned int & BitValue); unsigned int Get8Bits(); bool Peek7Bits(unsigned int & Value); void SkipBits(unsigned int BitCount); diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 31e9981..60e2167 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3787,15 +3787,13 @@ static DWORD TestReplaceFile(LPCTSTR szMpqPlainName, LPCTSTR szFilePlainName, LP static void Test_PlayingSpace() { -/* HANDLE hMpq = NULL; - if(SFileOpenArchive(_T("e:\\poc17"), 0, 0, &hMpq)) + if(SFileOpenArchive(_T("e:\\poc18"), 0, 0, &hMpq)) { - SFileCompactArchive(hMpq, _T("e:\\Ladik\\Incoming\\poc17"), true); + SFileCompactArchive(hMpq, _T("e:\\Ladik\\Incoming\\poc18"), true); SFileCloseArchive(hMpq); } -*/ } //----------------------------------------------------------------------------- -- cgit v1.2.3 From c4e3490d729ba42e92803b7f2ef90ed86b0b0eca Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 20:21:38 +0200 Subject: Added buffer overflow checks to the Sparse decompression (https://github.com/ladislav-zezula/StormLib/issues/337) --- src/huffman/huff.h | 2 +- src/sparse/sparse.cpp | 5 +++++ test/StormTest.cpp | 4 +++- 3 files changed, 9 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/src/huffman/huff.h b/src/huffman/huff.h index e6f9e42..b2c6370 100644 --- a/src/huffman/huff.h +++ b/src/huffman/huff.h @@ -29,7 +29,7 @@ class TInputStream TInputStream(void * pvInBuffer, size_t cbInBuffer); bool Get1Bit(unsigned int & BitValue); - unsigned int Get8Bits(); + bool Get8Bits(unsigned int & ByteValue); bool Peek7Bits(unsigned int & Value); void SkipBits(unsigned int BitCount); diff --git a/src/sparse/sparse.cpp b/src/sparse/sparse.cpp index 6d1b621..6cf2df2 100644 --- a/src/sparse/sparse.cpp +++ b/src/sparse/sparse.cpp @@ -261,7 +261,12 @@ int DecompressSparse(void * pvOutBuffer, int * pcbOutBuffer, void * pvInBuffer, // If highest bit, it means that that normal data follow if(OneByte & 0x80) { + // Check the length of one chunk. Check for overflows cbChunkSize = (OneByte & 0x7F) + 1; + if((pbInBuffer + cbChunkSize) > pbInBufferEnd) + return 0; + + // Copy the chunk. Make sure that the buffer won't overflow cbChunkSize = (cbChunkSize < cbOutBuffer) ? cbChunkSize : cbOutBuffer; memcpy(pbOutBuffer, pbInBuffer, cbChunkSize); pbInBuffer += cbChunkSize; diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 60e2167..721ee19 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3789,7 +3789,7 @@ static void Test_PlayingSpace() { HANDLE hMpq = NULL; - if(SFileOpenArchive(_T("e:\\poc18"), 0, 0, &hMpq)) + if(SFileOpenArchive(_T("e:\\poc21"), 0, 0, &hMpq)) { SFileCompactArchive(hMpq, _T("e:\\Ladik\\Incoming\\poc18"), true); SFileCloseArchive(hMpq); @@ -4008,6 +4008,8 @@ static const TEST_INFO1 Test_OpenMpqs[] = {_T("pocs/MPQ_2024_07_InvalidBitmapFooter.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, {_T("pocs/MPQ_2024_08_InvalidSectorSize.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, {_T("pocs/MPQ_2024_09_InvalidSectorSize.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_10_HuffDecompressError.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_10_SparseDecompressError.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, // Correct or damaged archives {_T("MPQ_1997_v1_Diablo1_DIABDAT.MPQ"), NULL, "554b538541e42170ed41cb236483489e", 2910, &TwoFilesD1}, // Base MPQ from Diablo 1 -- cgit v1.2.3 From 5232da3f6ac96177db4d2d806c37c82fc664af8b Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 20:30:56 +0200 Subject: Fixed buffer overflow in https://github.com/ladislav-zezula/StormLib/issues/338 --- src/SBaseFileTable.cpp | 4 ++++ test/StormTest.cpp | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) (limited to 'test') diff --git a/src/SBaseFileTable.cpp b/src/SBaseFileTable.cpp index fc9418a..1ed8140 100644 --- a/src/SBaseFileTable.cpp +++ b/src/SBaseFileTable.cpp @@ -574,6 +574,10 @@ DWORD ConvertMpqHeaderToFormat4( // Fill the rest of the header with zeros memset((LPBYTE)pHeader + MPQ_HEADER_SIZE_V2, 0, sizeof(TMPQHeader) - MPQ_HEADER_SIZE_V2); + // Check position of the Hi-block table + if(pHeader->HiBlockTablePos64 > FileSize) + return ERROR_FILE_CORRUPT; + // Calculate the expected hash table size pHeader->HashTableSize64 = (pHeader->dwHashTableSize * sizeof(TMPQHash)); HashTablePos64 = MAKE_OFFSET64(pHeader->wHashTablePosHi, pHeader->dwHashTablePos); diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 721ee19..3c8051e 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3789,9 +3789,8 @@ static void Test_PlayingSpace() { HANDLE hMpq = NULL; - if(SFileOpenArchive(_T("e:\\poc21"), 0, 0, &hMpq)) + if(SFileOpenArchive(_T("e:\\poc24"), 0, 0, &hMpq)) { - SFileCompactArchive(hMpq, _T("e:\\Ladik\\Incoming\\poc18"), true); SFileCloseArchive(hMpq); } } @@ -4010,6 +4009,7 @@ static const TEST_INFO1 Test_OpenMpqs[] = {_T("pocs/MPQ_2024_09_InvalidSectorSize.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, {_T("pocs/MPQ_2024_10_HuffDecompressError.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, {_T("pocs/MPQ_2024_10_SparseDecompressError.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, + {_T("pocs/MPQ_2024_11_HiBlockTablePosInvalid.mpq"), NULL, "--------------------------------", TFLG_WILL_FAIL}, // Correct or damaged archives {_T("MPQ_1997_v1_Diablo1_DIABDAT.MPQ"), NULL, "554b538541e42170ed41cb236483489e", 2910, &TwoFilesD1}, // Base MPQ from Diablo 1 -- cgit v1.2.3 From b47bb8b1909e35e086cff00b64e1b0b60d68156f Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 21:24:48 +0200 Subject: Regression tests complete --- src/SBaseFileTable.cpp | 2 +- test/StormTest.cpp | 36 +++++++++++++++++++++++++++++------- 2 files changed, 30 insertions(+), 8 deletions(-) (limited to 'test') diff --git a/src/SBaseFileTable.cpp b/src/SBaseFileTable.cpp index 1ed8140..0461be2 100644 --- a/src/SBaseFileTable.cpp +++ b/src/SBaseFileTable.cpp @@ -574,7 +574,7 @@ DWORD ConvertMpqHeaderToFormat4( // Fill the rest of the header with zeros memset((LPBYTE)pHeader + MPQ_HEADER_SIZE_V2, 0, sizeof(TMPQHeader) - MPQ_HEADER_SIZE_V2); - // Check position of the Hi-block table + // Check position of the hi-block table if(pHeader->HiBlockTablePos64 > FileSize) return ERROR_FILE_CORRUPT; diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 3c8051e..fc84589 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3785,14 +3785,36 @@ static DWORD TestReplaceFile(LPCTSTR szMpqPlainName, LPCTSTR szFilePlainName, LP return dwErrCode; } +typedef uint8_t u8; +typedef uint16_t u16; +typedef uint32_t u32; +typedef ULONGLONG u64; +typedef unsigned int usize; +typedef char i8; +typedef short i16; +typedef int i32; +typedef LONGLONG i64; +typedef int isize; +typedef float f32; +typedef double f64; + static void Test_PlayingSpace() { - HANDLE hMpq = NULL; - - if(SFileOpenArchive(_T("e:\\poc24"), 0, 0, &hMpq)) - { - SFileCloseArchive(hMpq); - } +/* + i8 v0_tmp[] = {3, 31, -86, 93, 105, -118, -9, -9, 65, 31, 111, 123, -36, 45, -4, -113, 70, 111, -118, 65, 31, 122, 127, 6, -97, 36, 69, 117, 67, 106, -37, 64, 41, 64, -18, 120, -48, -61, -121, 43, 66, 31, 37, -6, -33, 40, 93, -98, 38, 106, 62, -71, 44, -110, 122, 37, 37, -50, -65, 98, 80, 12, 89, 74, -89, 21, -42, -99, -24, 112, 89, 83, 87, -30, 65, 103, -46, -118, 64, 96, 89, -1, -77, 4, 63, 102, -69, -120, -40, 66, -64, 16, 12, 29, -18, -121, 98, -41, 47, 68, 28, -28, 116, -79, 113, -77, -35, -54, -25, -5, 19, -2, 15, 62, 55, -59, -21, -28, -96, -94, -118, 19, 125, 102, 54, 20, 70, 0,}; // pvOutBuffer + i8 * v0 = (i8 *)HeapAlloc(GetProcessHeap(), 0, sizeof(v0_tmp)); + memcpy(v0, v0_tmp, sizeof v0_tmp); + i8 * v1 = v0; // pvOutBuffer + i32 v2_tmp[] = {129, 0,}; // pcbOutBuffer + i32 * v2 = (i32 *)HeapAlloc(GetProcessHeap(), 0, sizeof(v2_tmp)); + memcpy(v2, v2_tmp, sizeof v2_tmp); + i32 * v3 = v2; // pcbOutBuffer + i8 v4_tmp[] = {38, 17, 14, -64, 78, 79, -10, 115, 120, 103, 94, 9, 6, 21, 25, 43, 127, -17, -115, 118, 53, 76, 13, -105, 39, -11, -12, -35, -74, -114, -112, 75, 54, -100, -44, 68, 61, 7, 85, 96, 103, 103, 106, -95, -85, -113, -78, -78, 43, -39, 59, -58, -15, 35, -63, 125, -5, -13, 110, -66, -39, -97, -23, 37, -25, -105, -57, -128, 77, 7, 54, 95, 116, -128, -114, 13, -54, -17, 12, 126, -102, -83, 101, -103, -17, 78, 40, -103, -75, -55, -86, 23, 24, 87, -73, -123, 107, 112, -30, -77, -16, 93, -49, 84, -109, 78, 106, 62, -68, -61, 4, -121, 112, 94, 36, -39, 9, -59, 123, -49, -91, -111, -2, 32, 117, 30, 23, 0, 0,}; // pvInBuffer + i8 * v4 = (i8 *)HeapAlloc(GetProcessHeap(), 0, sizeof(v4_tmp)); + memcpy(v4, v4_tmp, sizeof v4_tmp); + + SCompCompress(v1, v3, v4, 128, 8, 122, 57); // $target +*/ } //----------------------------------------------------------------------------- @@ -4036,7 +4058,7 @@ static const TEST_INFO1 Test_OpenMpqs[] = {_T("MPQ_2023_v1_BroodWarMap.scx"), NULL, "dd3afa3c2f5e562ce3ca91c0c605a71f", 3}, // Brood War map from StarCraft: Brood War 1.16 {_T("MPQ_2023_v1_Volcanis.scm"), NULL, "522c89ca96d6736427b01f7c80dd626f", 3}, // Map modified with unusual file compression: ZLIB+Huffman {_T("MPQ_2023_v4_UTF8.s2ma"), NULL, "97b7a686650f3307d135e1d1b017a36a", 67}, // Map contaning files with Chinese names (UTF8-encoded) - {_T("MPQ_2023_v1_GreenTD.w3x"), NULL, "477af4ddf11eead1412d7c87cb81b530", 2004}, // Corrupt sector checksum table in file #A0 + {_T("MPQ_2023_v1_GreenTD.w3x"), NULL, "a8d91fc4e52d7c21ff7feb498c74781a", 2004}, // Corrupt sector checksum table in file #A0 {_T("MPQ_2023_v4_1F644C5A.SC2Replay"), NULL, "b225828ffbf5037553e6a1290187caab", 17}, // Corrupt patch info of the "(attributes)" file {_T(""), NULL, "67faeffd0c0aece205ac8b7282d8ad8e", 4697, &MpqUtf8}, // Chinese name of the MPQ -- cgit v1.2.3 From 4e09d09686df70efc1c16c2bb41f716c6ce4fca4 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 21:42:04 +0200 Subject: Fixed page fault from https://github.com/ladislav-zezula/StormLib/issues/343 --- src/SFileGetFileInfo.cpp | 2 ++ test/StormTest.cpp | 33 ++++++++++++++++++++------------- 2 files changed, 22 insertions(+), 13 deletions(-) (limited to 'test') diff --git a/src/SFileGetFileInfo.cpp b/src/SFileGetFileInfo.cpp index 20f5e75..5dd997d 100644 --- a/src/SFileGetFileInfo.cpp +++ b/src/SFileGetFileInfo.cpp @@ -428,6 +428,8 @@ bool WINAPI SFileGetFileInfo( return GetInfo(pvFileInfo, cbFileInfo, &dwInt32Value, sizeof(DWORD), pcbLengthNeeded); case SFileInfoFileIndex: + if(hf->ha == NULL) + return GetInfo_ReturnError(ERROR_INVALID_PARAMETER); dwInt32Value = (DWORD)(pFileEntry - hf->ha->pFileTable); return GetInfo(pvFileInfo, cbFileInfo, &dwInt32Value, sizeof(DWORD), pcbLengthNeeded); diff --git a/test/StormTest.cpp b/test/StormTest.cpp index fc84589..76d1f3b 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3800,21 +3800,28 @@ typedef double f64; static void Test_PlayingSpace() { -/* - i8 v0_tmp[] = {3, 31, -86, 93, 105, -118, -9, -9, 65, 31, 111, 123, -36, 45, -4, -113, 70, 111, -118, 65, 31, 122, 127, 6, -97, 36, 69, 117, 67, 106, -37, 64, 41, 64, -18, 120, -48, -61, -121, 43, 66, 31, 37, -6, -33, 40, 93, -98, 38, 106, 62, -71, 44, -110, 122, 37, 37, -50, -65, 98, 80, 12, 89, 74, -89, 21, -42, -99, -24, 112, 89, 83, 87, -30, 65, 103, -46, -118, 64, 96, 89, -1, -77, 4, 63, 102, -69, -120, -40, 66, -64, 16, 12, 29, -18, -121, 98, -41, 47, 68, 28, -28, 116, -79, 113, -77, -35, -54, -25, -5, 19, -2, 15, 62, 55, -59, -21, -28, -96, -94, -118, 19, 125, 102, 54, 20, 70, 0,}; // pvOutBuffer - i8 * v0 = (i8 *)HeapAlloc(GetProcessHeap(), 0, sizeof(v0_tmp)); - memcpy(v0, v0_tmp, sizeof v0_tmp); - i8 * v1 = v0; // pvOutBuffer - i32 v2_tmp[] = {129, 0,}; // pcbOutBuffer - i32 * v2 = (i32 *)HeapAlloc(GetProcessHeap(), 0, sizeof(v2_tmp)); + void * v0 = NULL; // hMpq + void * v1 = NULL; // hMpqOrFile + u8 v2_tmp[] = {83, 0,}; // file_buf + u8 * v2 = (u8 *)HeapAlloc(GetProcessHeap(), 0, sizeof v2_tmp); memcpy(v2, v2_tmp, sizeof v2_tmp); - i32 * v3 = v2; // pcbOutBuffer - i8 v4_tmp[] = {38, 17, 14, -64, 78, 79, -10, 115, 120, 103, 94, 9, 6, 21, 25, 43, 127, -17, -115, 118, 53, 76, 13, -105, 39, -11, -12, -35, -74, -114, -112, 75, 54, -100, -44, 68, 61, 7, 85, 96, 103, 103, 106, -95, -85, -113, -78, -78, 43, -39, 59, -58, -15, 35, -63, 125, -5, -13, 110, -66, -39, -97, -23, 37, -25, -105, -57, -128, 77, 7, 54, 95, 116, -128, -114, 13, -54, -17, 12, 126, -102, -83, 101, -103, -17, 78, 40, -103, -75, -55, -86, 23, 24, 87, -73, -123, 107, 112, -30, -77, -16, 93, -49, 84, -109, 78, 106, 62, -68, -61, 4, -121, 112, 94, 36, -39, 9, -59, 123, -49, -91, -111, -2, 32, 117, 30, 23, 0, 0,}; // pvInBuffer - i8 * v4 = (i8 *)HeapAlloc(GetProcessHeap(), 0, sizeof(v4_tmp)); - memcpy(v4, v4_tmp, sizeof v4_tmp); - SCompCompress(v1, v3, v4, 128, 8, 122, 57); // $target -*/ + char * path_v3 = "e:\\hop-poc09"; + FILE * f_v3 = fopen(path_v3, "wb"); + fwrite(v2, sizeof v2_tmp, 1, f_v3); + fclose(f_v3); + + char * v3 = path_v3; // szFileName + u32 v4 = 4294967295; // dwSearchScope + void ** v5 = &v1; // phFile + i8 v6 = SFileOpenFileEx(v0, v3, v4, v5); // $relative + if(v6 == false) exit(1); + + enum _SFileInfoClass v8 = (enum _SFileInfoClass)48; // InfoClass + i8 * v9 = NULL; // pvFileInfo + u32 * v10 = NULL; // pcbLengthNeeded + u32 v11 = 0; // cbFileInfo + i8 v12 = SFileGetFileInfo(v1, v8, v9, v11, (LPDWORD)(v10)); // $target } //----------------------------------------------------------------------------- -- cgit v1.2.3 From 0f8c306a3e9dfa536e97e9e0678773c40ea13a52 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 21:45:52 +0200 Subject: Fixed page fault from https://github.com/ladislav-zezula/StormLib/issues/344 --- src/SFileGetFileInfo.cpp | 2 ++ test/StormTest.cpp | 26 ++++++++++++++------------ 2 files changed, 16 insertions(+), 12 deletions(-) (limited to 'test') diff --git a/src/SFileGetFileInfo.cpp b/src/SFileGetFileInfo.cpp index 5dd997d..1746fa0 100644 --- a/src/SFileGetFileInfo.cpp +++ b/src/SFileGetFileInfo.cpp @@ -452,6 +452,8 @@ bool WINAPI SFileGetFileInfo( return GetInfo(pvFileInfo, cbFileInfo, &hf->dwFileKey, sizeof(DWORD), pcbLengthNeeded); case SFileInfoEncryptionKeyRaw: + if(pFileEntry == NULL) + return GetInfo_ReturnError(ERROR_INVALID_PARAMETER); dwInt32Value = hf->dwFileKey; if(pFileEntry->dwFlags & MPQ_FILE_KEY_V2) dwInt32Value = (dwInt32Value ^ pFileEntry->dwFileSize) - (DWORD)hf->MpqFilePos; diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 76d1f3b..88eed97 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3801,27 +3801,29 @@ typedef double f64; static void Test_PlayingSpace() { void * v0 = NULL; // hMpq - void * v1 = NULL; // hMpqOrFile - u8 v2_tmp[] = {83, 0,}; // file_buf - u8 * v2 = (u8 *)HeapAlloc(GetProcessHeap(), 0, sizeof v2_tmp); + void * v1 = NULL; // hFile + u8 v2_tmp[] = {181, 0,}; // file_buf + u8 * v2 = (u8 *)malloc(sizeof v2_tmp); memcpy(v2, v2_tmp, sizeof v2_tmp); - - char * path_v3 = "e:\\hop-poc09"; + char * path_v3 = "e:\\hop-poc10"; FILE * f_v3 = fopen(path_v3, "wb"); fwrite(v2, sizeof v2_tmp, 1, f_v3); fclose(f_v3); - char * v3 = path_v3; // szFileName u32 v4 = 4294967295; // dwSearchScope void ** v5 = &v1; // phFile i8 v6 = SFileOpenFileEx(v0, v3, v4, v5); // $relative if(v6 == false) exit(1); - - enum _SFileInfoClass v8 = (enum _SFileInfoClass)48; // InfoClass - i8 * v9 = NULL; // pvFileInfo - u32 * v10 = NULL; // pcbLengthNeeded - u32 v11 = 0; // cbFileInfo - i8 v12 = SFileGetFileInfo(v1, v8, v9, v11, (LPDWORD)(v10)); // $target + i8 * v8 = NULL; // pvData + u32 v9 = 254; // dwSize + u32 v10 = 11; // dwCompression + enum _SFileInfoClass v11 = (enum _SFileInfoClass)(55); // InfoClass + i8 * v12 = NULL; // pvFileInfo + u32 v13 = 0; // cbFileInfo + u32 * v14 = NULL; // pcbLengthNeeded + i8 v15 = SFileGetFileInfo(v1, v11, v12, v13, (LPDWORD)(v14)); // $relative + if(v15 == false) exit(1); + i8 v16 = SFileWriteFile(v1, v8, v9, v10); // $target } //----------------------------------------------------------------------------- -- cgit v1.2.3 From 7250eca739f060dd6984b9ea74a9fbb8b0a7c353 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Sun, 21 Apr 2024 22:33:13 +0200 Subject: Regression tests complete --- test/StormTest.cpp | 46 +++++++++------------------------------------- 1 file changed, 9 insertions(+), 37 deletions(-) (limited to 'test') diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 88eed97..1b13b89 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -3785,45 +3785,17 @@ static DWORD TestReplaceFile(LPCTSTR szMpqPlainName, LPCTSTR szFilePlainName, LP return dwErrCode; } -typedef uint8_t u8; -typedef uint16_t u16; -typedef uint32_t u32; -typedef ULONGLONG u64; -typedef unsigned int usize; -typedef char i8; -typedef short i16; -typedef int i32; -typedef LONGLONG i64; -typedef int isize; -typedef float f32; -typedef double f64; - static void Test_PlayingSpace() { - void * v0 = NULL; // hMpq - void * v1 = NULL; // hFile - u8 v2_tmp[] = {181, 0,}; // file_buf - u8 * v2 = (u8 *)malloc(sizeof v2_tmp); - memcpy(v2, v2_tmp, sizeof v2_tmp); - char * path_v3 = "e:\\hop-poc10"; - FILE * f_v3 = fopen(path_v3, "wb"); - fwrite(v2, sizeof v2_tmp, 1, f_v3); - fclose(f_v3); - char * v3 = path_v3; // szFileName - u32 v4 = 4294967295; // dwSearchScope - void ** v5 = &v1; // phFile - i8 v6 = SFileOpenFileEx(v0, v3, v4, v5); // $relative - if(v6 == false) exit(1); - i8 * v8 = NULL; // pvData - u32 v9 = 254; // dwSize - u32 v10 = 11; // dwCompression - enum _SFileInfoClass v11 = (enum _SFileInfoClass)(55); // InfoClass - i8 * v12 = NULL; // pvFileInfo - u32 v13 = 0; // cbFileInfo - u32 * v14 = NULL; // pcbLengthNeeded - i8 v15 = SFileGetFileInfo(v1, v11, v12, v13, (LPDWORD)(v14)); // $relative - if(v15 == false) exit(1); - i8 v16 = SFileWriteFile(v1, v8, v9, v10); // $target +/* + i8 v0_tmp[] = {5, 34, -58, 65, 113, -118, 76, 11, 40, 32, 27, 20, 83, 15, 22, 46, 25, -24, -77, -88, -70, -118, -58, 56, 55, -94, -69, 43, -87, -1, -70, 0,}; // pvFileInfo + i8 * v0 = (i8 *)malloc(sizeof v0_tmp); + memcpy(v0, v0_tmp, sizeof v0_tmp); + i8 * v1 = v0; // pvFileInfo + + enum _SFileInfoClass v2 = (enum _SFileInfoClass)(11); // InfoClass + i8 v3 = SFileFreeFileInfo(v1, v2); // $target +*/ } //----------------------------------------------------------------------------- -- cgit v1.2.3