Fixed heap overflow in handling of file patch

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.