From 8debce7eab1cfb7a145d592d757b75e7cac83610 Mon Sep 17 00:00:00 2001 From: Ladislav Zezula Date: Fri, 4 Aug 2023 11:19:49 +0200 Subject: Fixed heap overflow in handling of file patch --- make-msvc.bat | 2 +- src/SBaseCommon.cpp | 40 +++++++++++++++++++++++----------------- src/SFileAttributes.cpp | 5 ++++- src/StormCommon.h | 2 +- src/StormLib.h | 7 +++++-- test/StormTest.cpp | 1 + test/stormlib-test-001.txt | 1 + 7 files changed, 36 insertions(+), 22 deletions(-) diff --git a/make-msvc.bat b/make-msvc.bat index 2422490..6e3782d 100644 --- a/make-msvc.bat +++ b/make-msvc.bat @@ -8,7 +8,7 @@ set SAVE_PATH=%PATH% set LIB_NAME=StormLib :: Determine where the program files are, both for 64-bit and 32-bit Windows -if exist "%ProgramFiles%" set PROGRAM_FILES_X64=%ProgramFiles% +if exist "%ProgramW6432%" set PROGRAM_FILES_X64=%ProgramW6432% if exist "%ProgramFiles%" set PROGRAM_FILES_DIR=%ProgramFiles% if exist "%ProgramFiles(x86)%" set PROGRAM_FILES_DIR=%ProgramFiles(x86)% diff --git a/src/SBaseCommon.cpp b/src/SBaseCommon.cpp index b0029e0..b5b880e 100644 --- a/src/SBaseCommon.cpp +++ b/src/SBaseCommon.cpp @@ -1154,6 +1154,7 @@ DWORD AllocateSectorBuffer(TMPQFile * hf) DWORD AllocatePatchInfo(TMPQFile * hf, bool bLoadFromFile) { TMPQArchive * ha = hf->ha; + TPatchInfo * pPatchInfo; DWORD dwLength = sizeof(TPatchInfo); // The following conditions must be true @@ -1164,35 +1165,39 @@ __AllocateAndLoadPatchInfo: // Allocate space for patch header. Start with default size, // and if its size if bigger, then we reload them - hf->pPatchInfo = STORM_ALLOC(TPatchInfo, 1); - if(hf->pPatchInfo == NULL) + pPatchInfo = (TPatchInfo *)(STORM_ALLOC(BYTE, dwLength)); + if(pPatchInfo == NULL) return ERROR_NOT_ENOUGH_MEMORY; // Do we have to load the patch header from the file ? if(bLoadFromFile) { // Load the patch header - if(!FileStream_Read(ha->pStream, &hf->RawFilePos, hf->pPatchInfo, dwLength)) + if(!FileStream_Read(ha->pStream, &hf->RawFilePos, pPatchInfo, dwLength)) { - // Free the patch info - STORM_FREE(hf->pPatchInfo); - hf->pPatchInfo = NULL; + STORM_FREE(pPatchInfo); return GetLastError(); } // Perform necessary swapping - hf->pPatchInfo->dwLength = BSWAP_INT32_UNSIGNED(hf->pPatchInfo->dwLength); - hf->pPatchInfo->dwFlags = BSWAP_INT32_UNSIGNED(hf->pPatchInfo->dwFlags); - hf->pPatchInfo->dwDataSize = BSWAP_INT32_UNSIGNED(hf->pPatchInfo->dwDataSize); + pPatchInfo->dwLength = BSWAP_INT32_UNSIGNED(pPatchInfo->dwLength); + pPatchInfo->dwFlags = BSWAP_INT32_UNSIGNED(pPatchInfo->dwFlags); + pPatchInfo->dwDataSize = BSWAP_INT32_UNSIGNED(pPatchInfo->dwDataSize); + + // Do nothing if the patch info is not valid + if(!(pPatchInfo->dwFlags & MPQ_PATCH_INFO_VALID)) + { + STORM_FREE(pPatchInfo); + return ERROR_FILE_CORRUPT; + } // Verify the size of the patch header // If it's not default size, we have to reload them - if(hf->pPatchInfo->dwLength > dwLength) + if(pPatchInfo->dwLength > dwLength) { // Free the patch info - dwLength = hf->pPatchInfo->dwLength; - STORM_FREE(hf->pPatchInfo); - hf->pPatchInfo = NULL; + dwLength = pPatchInfo->dwLength; + STORM_FREE(pPatchInfo); // If the length is out of all possible ranges, fail the operation if(dwLength > 0x400) @@ -1201,16 +1206,17 @@ __AllocateAndLoadPatchInfo: } // Patch file data size according to the patch header - hf->dwDataSize = hf->pPatchInfo->dwDataSize; + hf->dwDataSize = pPatchInfo->dwDataSize; } else { - memset(hf->pPatchInfo, 0, dwLength); + memset(pPatchInfo, 0, dwLength); + pPatchInfo->dwLength = dwLength; + pPatchInfo->dwFlags = MPQ_PATCH_INFO_VALID; } // Save the final length to the patch header - hf->pPatchInfo->dwLength = dwLength; - hf->pPatchInfo->dwFlags = 0x80000000; + hf->pPatchInfo = pPatchInfo; return ERROR_SUCCESS; } diff --git a/src/SFileAttributes.cpp b/src/SFileAttributes.cpp index e22a09f..dbcc456 100644 --- a/src/SFileAttributes.cpp +++ b/src/SFileAttributes.cpp @@ -391,7 +391,10 @@ DWORD SAttrLoadAttributes(TMPQArchive * ha) pbAttrFile[cbAttrFile] = 0; // Load the entire file to memory - SFileReadFile(hFile, pbAttrFile, cbAttrFile, &dwBytesRead, NULL); + if(!SFileReadFile(hFile, pbAttrFile, cbAttrFile, &dwBytesRead, NULL)) + ha->dwFlags |= (GetLastError() == ERROR_FILE_CORRUPT) ? MPQ_FLAG_MALFORMED : 0; + + // Parse the (attributes) if(dwBytesRead == cbAttrFile) dwErrCode = LoadAttributesFile(ha, pbAttrFile, cbAttrFile); diff --git a/src/StormCommon.h b/src/StormCommon.h index 74b687e..34077fd 100644 --- a/src/StormCommon.h +++ b/src/StormCommon.h @@ -82,7 +82,7 @@ #endif //----------------------------------------------------------------------------- -// MTYPE definition - specifies what kind of MPQ is the map type +// MTYPE definition - specifies what kind of MPQ is the file typedef enum _MTYPE { diff --git a/src/StormLib.h b/src/StormLib.h index 4aa51c1..4d5992d 100644 --- a/src/StormLib.h +++ b/src/StormLib.h @@ -215,7 +215,7 @@ extern "C" { #define SFILE_INVALID_POS 0xFFFFFFFF #define SFILE_INVALID_ATTRIBUTES 0xFFFFFFFF -// Flags for SFileAddFile +// Flags for TMPQBlock::dwFlags #define MPQ_FILE_IMPLODE 0x00000100 // Implode method (By PKWARE Data Compression Library) #define MPQ_FILE_COMPRESS 0x00000200 // Compress methods (By multiple methods) #define MPQ_FILE_ENCRYPTED 0x00010000 // Indicates whether file is encrypted @@ -259,6 +259,9 @@ extern "C" { MPQ_FILE_FIX_KEY | \ MPQ_FILE_EXISTS) +// Flags for TPatchInfo::dwFlags +#define MPQ_PATCH_INFO_VALID 0x80000000 // Set if the patch info is valid + // 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 << 0x04 @@ -676,7 +679,7 @@ typedef struct _TMPQBlock typedef struct _TPatchInfo { DWORD dwLength; // Length of patch info header, in bytes - DWORD dwFlags; // Flags. 0x80000000 = MD5 (?) + DWORD dwFlags; // Flags. 0x80000000 = valid (?) DWORD dwDataSize; // Uncompressed size of the patch file BYTE md5[0x10]; // MD5 of the entire patch file after decompression diff --git a/test/StormTest.cpp b/test/StormTest.cpp index 2bd5ceb..99138e4 100755 --- a/test/StormTest.cpp +++ b/test/StormTest.cpp @@ -4115,6 +4115,7 @@ static const TEST_INFO Test_OpenMpqs[] = {_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_v4_1F644C5A.SC2Replay"), NULL, "b225828ffbf5037553e6a1290187caab", 17}, // Corrupt patch info of the "(attributes)" file // Protected archives {_T("MPQ_2002_v1_ProtectedMap_InvalidUserData.w3x"), NULL, "b900364cc134a51ddeca21a13697c3ca", 79}, diff --git a/test/stormlib-test-001.txt b/test/stormlib-test-001.txt index 1f2be43..91af3c2 100644 --- a/test/stormlib-test-001.txt +++ b/test/stormlib-test-001.txt @@ -45,6 +45,7 @@ TestReadingMpq (MPQ_2023_v1_BroodWarMap.scx) succeeded. TestReadingMpq (MPQ_2023_v1_Volcanis.scm) succeeded. TestReadingMpq (MPQ_2023_v4_UTF8.s2ma) succeeded. TestReadingMpq (MPQ_2023_v1_GreenTD.w3x) succeeded. +TestReadingMpq (MPQ_2023_v4_1F644C5A.SC2Replay) succeeded. TestReadingMpq (MPQ_2002_v1_ProtectedMap_InvalidUserData.w3x) succeeded. TestReadingMpq (MPQ_2002_v1_ProtectedMap_InvalidMpqFormat.w3x) succeeded. TestReadingMpq (MPQ_2002_v1_ProtectedMap_Spazzler.w3x) succeeded. -- cgit v1.2.3