From 7d65d87773e3e7e587ba98a325e85d9cef621943 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Thu, 28 Jan 2016 08:56:48 +0100 Subject: + Anti-integer overflow in calculating buffer for hash table, and position of the block table entry --- src/SBaseFileTable.cpp | 13 +++++++++++++ src/SFileOpenArchive.cpp | 7 +++++++ test/StormTest.cpp | 10 +++++++--- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/SBaseFileTable.cpp b/src/SBaseFileTable.cpp index ed0748d..d822dc7 100644 --- a/src/SBaseFileTable.cpp +++ b/src/SBaseFileTable.cpp @@ -685,6 +685,12 @@ static TMPQHash * DefragmentHashTable( // Parse the hash table and move the entries to the begin of it for(pSource = pHashTable; pSource < pHashTableEnd; pSource++) { + // We need to mask out the upper 4 bits of the block table index. + // This is because it gets shifted out when calculating block table offset + // BlockTableOffset = pHash->dwBlockIndex * 0x10 + // Malformed MPQ maps may contain invalid entries + pSource->dwBlockIndex &= 0x0FFFFFFF; + // Check whether this is a valid hash table entry if(IsValidHashEntry1(ha, pSource, pBlockTable)) { @@ -768,6 +774,12 @@ static int BuildFileTableFromBlockTable( DWORD dwBlockIndex = pHash->dwBlockIndex; DWORD dwNewIndex = pHash->dwBlockIndex; + // We need to mask out the upper 4 bits of the block table index. + // This is because it gets shifted out when calculating block table offset + // BlockTableOffset = pHash->dwBlockIndex * 0x10 + // Malformed MPQ maps may contain invalid entries + pHash->dwBlockIndex &= 0x0FFFFFFF; + // // We need to properly handle these cases: // - Multiple hash entries (same file name) point to the same block entry @@ -776,6 +788,7 @@ static int BuildFileTableFromBlockTable( // Ignore all hash table entries where: // - dwBlockIndex >= BlockTableSize // - Flags of the appropriate block table entry + // if(IsValidHashEntry1(ha, pHash, pBlockTable)) { diff --git a/src/SFileOpenArchive.cpp b/src/SFileOpenArchive.cpp index f9469fb..6a249f2 100644 --- a/src/SFileOpenArchive.cpp +++ b/src/SFileOpenArchive.cpp @@ -365,6 +365,13 @@ bool WINAPI SFileOpenArchive( ha->pUserData = NULL; } + // Anti-overflow. If the hash table size in the header is + // higher than 0x10000000, it would overflow in 32-bit version + // Observed in the malformed Warcraft III maps + // Example map: MPQ_2016_v1_ProtectedMap_TableSizeOverflow.w3x + ha->pHeader->dwHashTableSize &= 0x0FFFFFFF; + ha->pHeader->dwBlockTableSize &= 0x0FFFFFFF; + // Both MPQ_OPEN_NO_LISTFILE or MPQ_OPEN_NO_ATTRIBUTES trigger read only mode if(dwFlags & (MPQ_OPEN_NO_LISTFILE | MPQ_OPEN_NO_ATTRIBUTES)) ha->dwFlags |= MPQ_FLAG_READ_ONLY; diff --git a/test/StormTest.cpp b/test/StormTest.cpp index c6908dc..2926f49 100644 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -4287,7 +4287,11 @@ int main(int argc, char * argv[]) // Open an protected map if(nError == ERROR_SUCCESS) nError = TestOpenArchive("MPQ_2015_v1_flem1.w3x"); - +*/ + // Open another protected map + if(nError == ERROR_SUCCESS) + nError = TestOpenArchive("MPQ_2016_v1_ProtectedMap_TableSizeOverflow.w3x"); +/* // Open the multi-file archive with wrong prefix to see how StormLib deals with it if(nError == ERROR_SUCCESS) nError = TestOpenArchive_WillFail("flat-file://streaming/model.MPQ.0"); @@ -4303,7 +4307,7 @@ int main(int argc, char * argv[]) // Test on an archive that has been invalidated by extending an old valid MPQ if(nError == ERROR_SUCCESS) nError = TestOpenArchive_Corrupt("MPQ_2013_vX_Battle.net.MPQ"); -*/ + // Open a patched archive if(nError == ERROR_SUCCESS) nError = TestOpenArchive_Patched(PatchList_WoW_OldWorld13286, "OldWorld\\World\\Model.blob", 2); @@ -4343,7 +4347,7 @@ int main(int argc, char * argv[]) // Open a patched archive if(nError == ERROR_SUCCESS) nError = TestOpenArchive_Patched(PatchList_HS_6898_enGB, "Hearthstone_Data\\Managed\\Assembly-Csharp.dll", 10); -/* + // Check the opening archive for read-only if(nError == ERROR_SUCCESS) nError = TestOpenArchive_ReadOnly("MPQ_1997_v1_Diablo1_DIABDAT.MPQ", true); -- cgit v1.2.3