Robustify path handling and prevent buffer overflows

Core/GameEngine/Source/Common/System/MiniDumper.cpp

@@ -146,9 +146,12 @@ void MiniDumper::Initialize(const AsciiString& userDirPath)
 	}
 
 	DWORD executableSize = ::GetModuleFileNameW(NULL, m_executablePath, ARRAY_SIZE(m_executablePath));
-	if (executableSize == 0 || executableSize == ARRAY_SIZE(m_executablePath))
+	// GetModuleFileNameW returns 0 on failure, or buffer size if path was truncated
+	// In Windows Vista+, if buffer is too small, it returns the buffer size and sets ERROR_INSUFFICIENT_BUFFER
+	// We must check both conditions to ensure we have a valid, non-truncated path
+	if (executableSize == 0 || executableSize >= ARRAY_SIZE(m_executablePath))
 	{
-		DEBUG_LOG(("MiniDumper::Initialize: Could not get executable file name. Returned value=%u", executableSize));
+		DEBUG_LOG(("MiniDumper::Initialize: Could not get executable file name or path was truncated. Returned value=%u, Buffer size=%u", executableSize, ARRAY_SIZE(m_executablePath)));
 		return;
 	}
 

Generals/Code/GameEngine/Source/Common/GlobalData.cpp

@@ -1169,12 +1169,39 @@ void GlobalData::parseGameDataDefinition( INI* ini )
 	char temp[_MAX_PATH];
 	if (::SHGetSpecialFolderPath(NULL, temp, CSIDL_PERSONAL, true))
 	{
-		if (temp[strlen(temp)-1] != '\\')
-			strcat(temp, "\\");
-		strcat(temp, TheWritableGlobalData->m_userDataLeafName.str());
-		strcat(temp, "\\");
-		CreateDirectory(temp, NULL);
-		TheWritableGlobalData->m_userDataDir = temp;
+		size_t tempLen = strlen(temp);
+
+		// Ensure we have enough space for backslash + user data leaf name + backslash + null terminator
+		if (tempLen > 0 && tempLen < _MAX_PATH - 1)
+		{
+			// Add trailing backslash if missing
+			if (temp[tempLen - 1] != '\\')
+			{
+				if (tempLen + 1 < _MAX_PATH)
+				{
+					temp[tempLen] = '\\';
+					temp[tempLen + 1] = '\0';
+					tempLen++;
+				}
+				else
+				{
+					// Path too long, cannot proceed
+					return;
+				}
+			}
+
+			// Check if we have enough space to append the user data leaf name
+			const char* leafName = TheWritableGlobalData->m_userDataLeafName.str();
+			size_t leafLen = strlen(leafName);
+
+			if (tempLen + leafLen + 2 < _MAX_PATH) // +2 for backslash and null terminator
+			{
+				strcat(temp, leafName);
+				strcat(temp, "\\");
+				CreateDirectory(temp, NULL);
+				TheWritableGlobalData->m_userDataDir = temp;
+			}
+		}
 	}
 
 	// override INI values with user preferences

Generals/Code/Main/WinMain.cpp

@@ -795,12 +795,22 @@ Int APIENTRY WinMain( HINSTANCE hInstance, HINSTANCE hPrevInstance,
 
 		/// @todo remove this force set of working directory later
 		Char buffer[ _MAX_PATH ];
-		GetModuleFileName( NULL, buffer, sizeof( buffer ) );
-		if (Char *pEnd = strrchr(buffer, '\\'))
+		DWORD pathLen = GetModuleFileName( NULL, buffer, sizeof( buffer ) );
+
+		// Check if GetModuleFileName succeeded and path wasn't truncated
+		if (pathLen > 0 && pathLen < sizeof( buffer ))
 		{
-			*pEnd = 0;
+			if (Char *pEnd = strrchr(buffer, '\\'))
+			{
+				*pEnd = 0;
+
+				// Only set current directory if the resulting path is valid
+				if (strlen(buffer) > 0)
+				{
+					::SetCurrentDirectory(buffer);
+				}
+			}
 		}
-		::SetCurrentDirectory(buffer);
 
 
 		#ifdef RTS_DEBUG

GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp

@@ -1049,26 +1049,33 @@ GlobalData::GlobalData()
   char temp[_MAX_PATH + 1];
   if (::SHGetSpecialFolderPath(NULL, temp, CSIDL_PERSONAL, true))
   {
-    AsciiString myDocumentsDirectory = temp;
+    // Ensure null termination and validate the path wasn't truncated
+    temp[_MAX_PATH] = '\0';
+    size_t tempLen = strlen(temp);
+
+    if (tempLen > 0 && tempLen < _MAX_PATH)
+    {
+      AsciiString myDocumentsDirectory = temp;
 
-    if (myDocumentsDirectory.getCharAt(myDocumentsDirectory.getLength() -1) != '\\')
-      myDocumentsDirectory.concat( '\\' );
+      if (myDocumentsDirectory.getCharAt(myDocumentsDirectory.getLength() -1) != '\\')
+        myDocumentsDirectory.concat( '\\' );
 
-    AsciiString leafName;
+      AsciiString leafName;
 
-    if ( !GetStringFromRegistry( "", "UserDataLeafName", leafName ) )
-    {
-      // Use something, anything
-      // [MH] had to remove this, otherwise mapcache build step won't run... DEBUG_CRASH( ( "Could not find registry key UserDataLeafName; defaulting to \"Command and Conquer Generals Zero Hour Data\" " ) );
-      leafName = "Command and Conquer Generals Zero Hour Data";
-    }
+      if ( !GetStringFromRegistry( "", "UserDataLeafName", leafName ) )
+      {
+        // Use something, anything
+        // [MH] had to remove this, otherwise mapcache build step won't run... DEBUG_CRASH( ( "Could not find registry key UserDataLeafName; defaulting to \"Command and Conquer Generals Zero Hour Data\" " ) );
+        leafName = "Command and Conquer Generals Zero Hour Data";
+      }
 
-    myDocumentsDirectory.concat( leafName );
-    if (myDocumentsDirectory.getCharAt( myDocumentsDirectory.getLength() - 1) != '\\')
-      myDocumentsDirectory.concat( '\\' );
+      myDocumentsDirectory.concat( leafName );
+      if (myDocumentsDirectory.getCharAt( myDocumentsDirectory.getLength() - 1) != '\\')
+        myDocumentsDirectory.concat( '\\' );
 
-    CreateDirectory(myDocumentsDirectory.str(), NULL);
-    m_userDataDir = myDocumentsDirectory;
+      CreateDirectory(myDocumentsDirectory.str(), NULL);
+      m_userDataDir = myDocumentsDirectory;
+    }
   }
 
 	//-allAdvice feature

GeneralsMD/Code/Main/WinMain.cpp

@@ -856,12 +856,22 @@ Int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
 
 		/// @todo remove this force set of working directory later
 		Char buffer[_MAX_PATH];
-		GetModuleFileName(NULL, buffer, sizeof(buffer));
-		if (Char* pEnd = strrchr(buffer, '\\'))
+		DWORD pathLen = GetModuleFileName(NULL, buffer, sizeof(buffer));
+
+		// Check if GetModuleFileName succeeded and path wasn't truncated
+		if (pathLen > 0 && pathLen < sizeof(buffer))
 		{
-			*pEnd = 0;
+			if (Char* pEnd = strrchr(buffer, '\\'))
+			{
+				*pEnd = 0;
+
+				// Only set current directory if the resulting path is valid
+				if (strlen(buffer) > 0)
+				{
+					::SetCurrentDirectory(buffer);
+				}
+			}
 		}
-		::SetCurrentDirectory(buffer);
 
 
 #ifdef RTS_DEBUG