From a0d5c398bf4decc8c3c9579772ba00991edebb06 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Wed, 11 Jun 2025 23:43:48 +0200 Subject: [PATCH 1/9] Add normalizePath function to file system. --- .../Code/GameEngine/Include/Common/FileSystem.h | 1 + .../GameEngine/Include/Common/LocalFileSystem.h | 1 + .../GameEngine/Source/Common/System/FileSystem.cpp | 8 ++++++++ .../Include/StdDevice/Common/StdLocalFileSystem.h | 1 + .../Win32Device/Common/Win32LocalFileSystem.h | 1 + .../Source/StdDevice/Common/StdLocalFileSystem.cpp | 11 +++++++++++ .../Win32Device/Common/Win32LocalFileSystem.cpp | 13 +++++++++++++ 7 files changed, 36 insertions(+) diff --git a/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h b/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h index 381658cfc2d..05a6d6e7975 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h @@ -140,6 +140,7 @@ class FileSystem : public SubsystemInterface Bool areMusicFilesOnCD(); void loadMusicFilesFromCD(); void unloadMusicFilesFromCD(); + AsciiString normalizePath(const AsciiString& path) const; ///< normalizes a file path. The path can refer to a directory. File path must be absolute, but does not need to exist. Returns an empty string on failure. protected: mutable std::map m_fileExist; }; diff --git a/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h b/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h index 0c54a4deba8..fdea2ee9032 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h @@ -50,6 +50,7 @@ class LocalFileSystem : public SubsystemInterface virtual void getFileListInDirectory(const AsciiString& currentDirectory, const AsciiString& originalDirectory, const AsciiString& searchName, FilenameList &filenameList, Bool searchSubdirectories) const = 0; ///< search the given directory for files matching the searchName (egs. *.ini, *.rep). Possibly search subdirectories. virtual Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const = 0; ///< see FileSystem.h virtual Bool createDirectory(AsciiString directory) = 0; ///< see FileSystem.h + virtual AsciiString normalizePath(const AsciiString& filePath) const; ///< see FileSystem.h protected: }; diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp index 02f4b95abfd..b08d79ee98c 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp @@ -339,3 +339,11 @@ void FileSystem::unloadMusicFilesFromCD() TheArchiveFileSystem->closeArchiveFile( MUSIC_BIG ); } + +//============================================================================ +// FileSystem::normalizePath +//============================================================================ +AsciiString FileSystem::normalizePath(const AsciiString& path) const +{ + return TheLocalFileSystem->normalizePath(path); +} \ No newline at end of file diff --git a/GeneralsMD/Code/GameEngineDevice/Include/StdDevice/Common/StdLocalFileSystem.h b/GeneralsMD/Code/GameEngineDevice/Include/StdDevice/Common/StdLocalFileSystem.h index 3008120f68e..b0409e59450 100644 --- a/GeneralsMD/Code/GameEngineDevice/Include/StdDevice/Common/StdLocalFileSystem.h +++ b/GeneralsMD/Code/GameEngineDevice/Include/StdDevice/Common/StdLocalFileSystem.h @@ -49,6 +49,7 @@ class StdLocalFileSystem : public LocalFileSystem virtual Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const; virtual Bool createDirectory(AsciiString directory); + virtual AsciiString normalizePath(const AsciiString& filePath) const; protected: }; diff --git a/GeneralsMD/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h b/GeneralsMD/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h index 310a00b9a37..c2c1c1cd7b7 100644 --- a/GeneralsMD/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h +++ b/GeneralsMD/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h @@ -49,6 +49,7 @@ class Win32LocalFileSystem : public LocalFileSystem virtual Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const; virtual Bool createDirectory(AsciiString directory); + virtual AsciiString normalizePath(const AsciiString& filePath) const; protected: }; diff --git a/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp b/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp index 24ec744934b..f815f5abfa3 100644 --- a/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp +++ b/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp @@ -339,3 +339,14 @@ Bool StdLocalFileSystem::createDirectory(AsciiString directory) } return result; } + +AsciiString StdLocalFileSystem::normalizePath(const AsciiString& filePath) const +{ + std::string nonNormalized(filePath.str()); +#ifndef _WIN32 + // Replace backslashes with forward slashes on unix + std::replace(unNormalized.begin(), unNormalized.end(), '\\', '/'); +#endif + std::filesystem::path pathNonNormalized(nonNormalized); + return AsciiString(pathNonNormalized.lexically_normal().string().c_str()); +} diff --git a/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp b/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp index 34262dbb825..37e4f523502 100644 --- a/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp +++ b/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp @@ -214,3 +214,16 @@ Bool Win32LocalFileSystem::createDirectory(AsciiString directory) } return FALSE; } + +AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) const +{ + TCHAR normalizeBuffer[MAX_PATH]; + DWORD charsWritten = GetFullPathName(filePath.str(), MAX_PATH, normalizeBuffer, NULL); + if (charsWritten == 0 || charsWritten > MAX_PATH) + { + // empty string is used as a non-success + return AsciiString::TheEmptyString; + } + + return AsciiString(normalizeBuffer); +} From ef606c8ea77bc7c1c7578e2cbef23c181a2f43d8 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Wed, 11 Jun 2025 23:46:43 +0200 Subject: [PATCH 2/9] Ensure portable map paths are contained within their intended directories. --- .../GameEngine/Include/Common/GameState.h | 2 + .../Include/Common/LocalFileSystem.h | 2 +- .../Common/System/SaveGame/GameState.cpp | 53 ++++++++++++++++++- .../Source/GameNetwork/ConnectionManager.cpp | 15 ++++-- .../Source/GameNetwork/GameInfo.cpp | 9 +++- 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Include/Common/GameState.h b/GeneralsMD/Code/GameEngine/Include/Common/GameState.h index 16a098c04ed..4e945c332ab 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/GameState.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/GameState.h @@ -195,6 +195,8 @@ class GameState : public SubsystemInterface, AsciiString getMapLeafName(const AsciiString& in) const; + Bool isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const; + protected: // snapshot methods diff --git a/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h b/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h index fdea2ee9032..fcec9499477 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/LocalFileSystem.h @@ -50,7 +50,7 @@ class LocalFileSystem : public SubsystemInterface virtual void getFileListInDirectory(const AsciiString& currentDirectory, const AsciiString& originalDirectory, const AsciiString& searchName, FilenameList &filenameList, Bool searchSubdirectories) const = 0; ///< search the given directory for files matching the searchName (egs. *.ini, *.rep). Possibly search subdirectories. virtual Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const = 0; ///< see FileSystem.h virtual Bool createDirectory(AsciiString directory) = 0; ///< see FileSystem.h - virtual AsciiString normalizePath(const AsciiString& filePath) const; ///< see FileSystem.h + virtual AsciiString normalizePath(const AsciiString& filePath) const = 0; ///< see FileSystem.h protected: }; diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index fb288a2e3b5..606c6595446 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -783,7 +783,7 @@ AsciiString GameState::getFilePathInSaveDirectory(const AsciiString& leaf) const //------------------------------------------------------------------------------------------------- Bool GameState::isInSaveDirectory(const AsciiString& path) const { - return path.startsWithNoCase(getSaveDirectory()); + return isPathInDirectory(path, getSaveDirectory()); } // ------------------------------------------------------------------------------------------------ @@ -902,10 +902,13 @@ AsciiString GameState::realMapPathToPortableMapPath(const AsciiString& in) const AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const { AsciiString prefix; + // The directory where the real map path should be contained in. + AsciiString containingBasePath; if (in.startsWithNoCase(PORTABLE_SAVE)) { // the save dir ends with "\\" prefix = getSaveDirectory(); + containingBasePath = prefix; prefix.concat(getMapLeafName(in)); } else if (in.startsWithNoCase(PORTABLE_MAPS)) @@ -913,6 +916,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const // the map dir DOES NOT end with "\\", must add it prefix = TheMapCache->getMapDir(); prefix.concat("\\"); + containingBasePath = prefix; prefix.concat(getMapLeafAndDirName(in)); } else if (in.startsWithNoCase(PORTABLE_USER_MAPS)) @@ -920,6 +924,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const // the map dir DOES NOT end with "\\", must add it prefix = TheMapCache->getUserMapDir(); prefix.concat("\\"); + containingBasePath = prefix; prefix.concat(getMapLeafAndDirName(in)); } else @@ -927,12 +932,56 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const DEBUG_CRASH(("Map file was not found in any of the expected directories; this is impossible")); //throw INI_INVALID_DATA; // uncaught exceptions crash us. better to just use a bad path. - prefix = in; + return AsciiString::TheEmptyString; } + + if (!isPathInDirectory(prefix, containingBasePath)) + { + return AsciiString::TheEmptyString; + } + prefix.toLower(); return prefix; } +Bool GameState::isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const +{ + AsciiString filePathNormalized = TheFileSystem->normalizePath(testPath); + AsciiString basePathNormalized = TheFileSystem->normalizePath(basePath); + + if (basePathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize base directory path '%s'.", basePath.str())); + return false; + } + else if (filePathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize file path '%s'.", testPath.str())); + return false; + } +#ifdef _WIN32 + if (!basePathNormalized.endsWith("\\")) + { + basePathNormalized.concat("\\"); + } + + if (!filePathNormalized.startsWithNoCase(basePathNormalized)) +#else + if (!basePathNormalized.endsWith("/")) + { + basePathNormalized.concat("/"); + } + + if (!filePathNormalized.startsWith(basePathNormalized)) +#endif + { + DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').", testPath.str(), filePathNormalized.str(), basePath.str(), basePathNormalized.str())); + return false; + } + + return true; +} + // ------------------------------------------------------------------------------------------------ /** Does the save game file exist */ // ------------------------------------------------------------------------------------------------ diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp index 83f8bd5f144..88df0a0a06d 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp @@ -688,7 +688,16 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg) DEBUG_LOG(("%ls\n", log.str())); #endif - if (TheFileSystem->doesFileExist(msg->getRealFilename().str())) + AsciiString realFileName = msg->getRealFilename(); + DEBUG_ASSERTLOG((!realFileName.isEmpty(), "Got a file name transferred that failed to normalize: '%s'!", msg->getPortableFilename().str())); + + if (realFileName.isEmpty()) + { + // As the file name failed to normalize (in other words is bogus), do nothing and let the transfer time out. + return; + } + + if (TheFileSystem->doesFileExist(realFileName.str())) { DEBUG_LOG(("File exists already!\n")); //return; @@ -720,13 +729,13 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg) } #endif // COMPRESS_TARGAS - File *fp = TheFileSystem->openFile(msg->getRealFilename().str(), File::CREATE | File::BINARY | File::WRITE); + File *fp = TheFileSystem->openFile(realFileName.str(), File::CREATE | File::BINARY | File::WRITE); if (fp) { fp->write(buf, len); fp->close(); fp = NULL; - DEBUG_LOG(("Wrote %d bytes to file %s!\n",len,msg->getRealFilename().str())); + DEBUG_LOG(("Wrote %d bytes to file %s!\n", len, realFileName.str())); } else diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index ddc322f2518..5d7f230f122 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -1081,7 +1081,14 @@ Bool ParseAsciiStringToGameInfo(GameInfo *game, AsciiString options) mapName.concat(token); mapName.concat('.'); mapName.concat(TheMapCache->getMapExtension()); - mapName = TheGameState->portableMapPathToRealMapPath(mapName); + AsciiString realMapName = TheGameState->portableMapPathToRealMapPath(mapName); + if (realMapName.isEmpty()) + { + optionsOk = FALSE; + DEBUG_LOG(("ParseAsciiStringToGameInfo - saw bogus map name ('%s'); quitting\n", mapName.str())); + break; + } + mapName = realMapName; sawMap = true; DEBUG_LOG(("ParseAsciiStringToGameInfo - map name is %s\n", mapName.str())); } From 49b98c9e2d968dd3a3d025090030f34f5e6d564d Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Fri, 13 Jun 2025 20:21:26 +0200 Subject: [PATCH 3/9] Ensure DEBUG_* format strings end with newline. --- .../GameEngine/Source/Common/System/SaveGame/GameState.cpp | 6 +++--- .../GameEngine/Source/GameNetwork/ConnectionManager.cpp | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index 606c6595446..20e5301e619 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -951,12 +951,12 @@ Bool GameState::isPathInDirectory(const AsciiString& testPath, const AsciiString if (basePathNormalized.isEmpty()) { - DEBUG_CRASH(("Unable to normalize base directory path '%s'.", basePath.str())); + DEBUG_CRASH(("Unable to normalize base directory path '%s'.\n", basePath.str())); return false; } else if (filePathNormalized.isEmpty()) { - DEBUG_CRASH(("Unable to normalize file path '%s'.", testPath.str())); + DEBUG_CRASH(("Unable to normalize file path '%s'.\n", testPath.str())); return false; } #ifdef _WIN32 @@ -975,7 +975,7 @@ Bool GameState::isPathInDirectory(const AsciiString& testPath, const AsciiString if (!filePathNormalized.startsWith(basePathNormalized)) #endif { - DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').", testPath.str(), filePathNormalized.str(), basePath.str(), basePathNormalized.str())); + DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').\n", testPath.str(), filePathNormalized.str(), basePath.str(), basePathNormalized.str())); return false; } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp index 88df0a0a06d..302deceb7d5 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp @@ -689,7 +689,7 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg) #endif AsciiString realFileName = msg->getRealFilename(); - DEBUG_ASSERTLOG((!realFileName.isEmpty(), "Got a file name transferred that failed to normalize: '%s'!", msg->getPortableFilename().str())); + DEBUG_ASSERTLOG((!realFileName.isEmpty(), "Got a file name transferred that failed to normalize: '%s'!\n", msg->getPortableFilename().str())); if (realFileName.isEmpty()) { From 847724c62f28b00115bcd06ea73faae15796fa76 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Fri, 13 Jun 2025 21:32:31 +0200 Subject: [PATCH 4/9] Backport changes to Generals. --- .../GameEngine/Include/Common/FileSystem.h | 1 + .../GameEngine/Include/Common/GameState.h | 2 + .../Include/Common/LocalFileSystem.h | 1 + .../Source/Common/System/FileSystem.cpp | 8 +++ .../Common/System/SaveGame/GameState.cpp | 53 ++++++++++++++++++- .../Source/GameNetwork/ConnectionManager.cpp | 15 ++++-- .../Source/GameNetwork/GameInfo.cpp | 9 +++- .../Win32Device/Common/Win32LocalFileSystem.h | 1 + .../Common/Win32LocalFileSystem.cpp | 13 +++++ 9 files changed, 97 insertions(+), 6 deletions(-) diff --git a/Generals/Code/GameEngine/Include/Common/FileSystem.h b/Generals/Code/GameEngine/Include/Common/FileSystem.h index abd90f0f08b..a742788a6c4 100644 --- a/Generals/Code/GameEngine/Include/Common/FileSystem.h +++ b/Generals/Code/GameEngine/Include/Common/FileSystem.h @@ -136,6 +136,7 @@ class FileSystem : public SubsystemInterface Bool areMusicFilesOnCD(); void loadMusicFilesFromCD(); void unloadMusicFilesFromCD(); + AsciiString normalizePath(const AsciiString& path) const; ///< normalizes a file path. The path can refer to a directory. File path must be absolute, but does not need to exist. Returns an empty string on failure. protected: diff --git a/Generals/Code/GameEngine/Include/Common/GameState.h b/Generals/Code/GameEngine/Include/Common/GameState.h index 4d0a5a423cc..ea1979d04d5 100644 --- a/Generals/Code/GameEngine/Include/Common/GameState.h +++ b/Generals/Code/GameEngine/Include/Common/GameState.h @@ -195,6 +195,8 @@ class GameState : public SubsystemInterface, AsciiString getMapLeafName(const AsciiString& in) const; + Bool isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const; + protected: // snapshot methods diff --git a/Generals/Code/GameEngine/Include/Common/LocalFileSystem.h b/Generals/Code/GameEngine/Include/Common/LocalFileSystem.h index 3e58119975e..87e78f173ac 100644 --- a/Generals/Code/GameEngine/Include/Common/LocalFileSystem.h +++ b/Generals/Code/GameEngine/Include/Common/LocalFileSystem.h @@ -50,6 +50,7 @@ class LocalFileSystem : public SubsystemInterface virtual void getFileListInDirectory(const AsciiString& currentDirectory, const AsciiString& originalDirectory, const AsciiString& searchName, FilenameList &filenameList, Bool searchSubdirectories) const = 0; ///< search the given directory for files matching the searchName (egs. *.ini, *.rep). Possibly search subdirectories. virtual Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const = 0; ///< see FileSystem.h virtual Bool createDirectory(AsciiString directory) = 0; ///< see FileSystem.h + virtual AsciiString normalizePath(const AsciiString& filePath) const = 0; ///< see FileSystem.h protected: }; diff --git a/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp b/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp index 2a3d17e3444..49480d9ea69 100644 --- a/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp @@ -322,3 +322,11 @@ void FileSystem::unloadMusicFilesFromCD() TheArchiveFileSystem->closeArchiveFile( MUSIC_BIG ); } + +//============================================================================ +// FileSystem::normalizePath +//============================================================================ +AsciiString FileSystem::normalizePath(const AsciiString& path) const +{ + return TheLocalFileSystem->normalizePath(path); +} \ No newline at end of file diff --git a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index c4935f26c8c..e0016b988ec 100644 --- a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -783,7 +783,7 @@ AsciiString GameState::getFilePathInSaveDirectory(const AsciiString& leaf) const //------------------------------------------------------------------------------------------------- Bool GameState::isInSaveDirectory(const AsciiString& path) const { - return path.startsWithNoCase(getSaveDirectory()); + return isPathInDirectory(path, getSaveDirectory()); } // ------------------------------------------------------------------------------------------------ @@ -902,10 +902,13 @@ AsciiString GameState::realMapPathToPortableMapPath(const AsciiString& in) const AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const { AsciiString prefix; + // The directory where the real map path should be contained in. + AsciiString containingBasePath; if (in.startsWithNoCase(PORTABLE_SAVE)) { // the save dir ends with "\\" prefix = getSaveDirectory(); + containingBasePath = prefix; prefix.concat(getMapLeafName(in)); } else if (in.startsWithNoCase(PORTABLE_MAPS)) @@ -913,6 +916,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const // the map dir DOES NOT end with "\\", must add it prefix = TheMapCache->getMapDir(); prefix.concat("\\"); + containingBasePath = prefix; prefix.concat(getMapLeafAndDirName(in)); } else if (in.startsWithNoCase(PORTABLE_USER_MAPS)) @@ -920,6 +924,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const // the map dir DOES NOT end with "\\", must add it prefix = TheMapCache->getUserMapDir(); prefix.concat("\\"); + containingBasePath = prefix; prefix.concat(getMapLeafAndDirName(in)); } else @@ -927,12 +932,56 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const DEBUG_CRASH(("Map file was not found in any of the expected directories; this is impossible")); //throw INI_INVALID_DATA; // uncaught exceptions crash us. better to just use a bad path. - prefix = in; + return AsciiString::TheEmptyString; } + + if (!isPathInDirectory(prefix, containingBasePath)) + { + return AsciiString::TheEmptyString; + } + prefix.toLower(); return prefix; } +Bool GameState::isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const +{ + AsciiString filePathNormalized = TheFileSystem->normalizePath(testPath); + AsciiString basePathNormalized = TheFileSystem->normalizePath(basePath); + + if (basePathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize base directory path '%s'.\n", basePath.str())); + return false; + } + else if (filePathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize file path '%s'.\n", testPath.str())); + return false; + } +#ifdef _WIN32 + if (!basePathNormalized.endsWith("\\")) + { + basePathNormalized.concat("\\"); + } + + if (!filePathNormalized.startsWithNoCase(basePathNormalized)) +#else + if (!basePathNormalized.endsWith("/")) + { + basePathNormalized.concat("/"); + } + + if (!filePathNormalized.startsWith(basePathNormalized)) +#endif + { + DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').\n", testPath.str(), filePathNormalized.str(), basePath.str(), basePathNormalized.str())); + return false; + } + + return true; +} + // ------------------------------------------------------------------------------------------------ /** Does the save game file exist */ // ------------------------------------------------------------------------------------------------ diff --git a/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp b/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp index 3ed78617532..ce6714215e9 100644 --- a/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp +++ b/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp @@ -688,7 +688,16 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg) DEBUG_LOG(("%ls\n", log.str())); #endif - if (TheFileSystem->doesFileExist(msg->getRealFilename().str())) + AsciiString realFileName = msg->getRealFilename(); + DEBUG_ASSERTLOG((!realFileName.isEmpty(), "Got a file name transferred that failed to normalize: '%s'!\n", msg->getPortableFilename().str())); + + if (realFileName.isEmpty()) + { + // As the file name failed to normalize (in other words is bogus), do nothing and let the transfer time out. + return; + } + + if (TheFileSystem->doesFileExist(realFileName.str())) { DEBUG_LOG(("File exists already!\n")); //return; @@ -720,13 +729,13 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg) } #endif // COMPRESS_TARGAS - File *fp = TheFileSystem->openFile(msg->getRealFilename().str(), File::CREATE | File::BINARY | File::WRITE); + File *fp = TheFileSystem->openFile(realFileName.str(), File::CREATE | File::BINARY | File::WRITE); if (fp) { fp->write(buf, len); fp->close(); fp = NULL; - DEBUG_LOG(("Wrote %d bytes to file %s!\n",len,msg->getRealFilename().str())); + DEBUG_LOG(("Wrote %d bytes to file %s!\n", len, realFileName.str())); } else diff --git a/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 24501b3dcc5..a9f3096c74e 100644 --- a/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -1044,7 +1044,14 @@ Bool ParseAsciiStringToGameInfo(GameInfo *game, AsciiString options) mapName.concat(token); mapName.concat('.'); mapName.concat(TheMapCache->getMapExtension()); - mapName = TheGameState->portableMapPathToRealMapPath(mapName); + AsciiString realMapName = TheGameState->portableMapPathToRealMapPath(mapName); + if (realMapName.isEmpty()) + { + optionsOk = FALSE; + DEBUG_LOG(("ParseAsciiStringToGameInfo - saw bogus map name ('%s'); quitting\n", mapName.str())); + break; + } + mapName = realMapName; sawMap = true; DEBUG_LOG(("ParseAsciiStringToGameInfo - map name is %s\n", mapName.str())); } diff --git a/Generals/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h b/Generals/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h index 8caf0a48720..4d25d27a109 100644 --- a/Generals/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h +++ b/Generals/Code/GameEngineDevice/Include/Win32Device/Common/Win32LocalFileSystem.h @@ -49,6 +49,7 @@ class Win32LocalFileSystem : public LocalFileSystem virtual Bool getFileInfo(const AsciiString& filename, FileInfo *fileInfo) const; virtual Bool createDirectory(AsciiString directory); + virtual AsciiString normalizePath(const AsciiString& filePath) const; protected: }; diff --git a/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp b/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp index 3c274e4b9ed..0e517be5e5b 100644 --- a/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp +++ b/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp @@ -214,3 +214,16 @@ Bool Win32LocalFileSystem::createDirectory(AsciiString directory) } return FALSE; } + +AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) const +{ + TCHAR normalizeBuffer[MAX_PATH]; + DWORD charsWritten = GetFullPathName(filePath.str(), MAX_PATH, normalizeBuffer, NULL); + if (charsWritten == 0 || charsWritten > MAX_PATH) + { + // empty string is used as a non-success + return AsciiString::TheEmptyString; + } + + return AsciiString(normalizeBuffer); +} \ No newline at end of file From 9156830cdc6fda98b0693ee33691480984ea9d1b Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Tue, 17 Jun 2025 21:42:43 +0200 Subject: [PATCH 5/9] Moved isPathInDirectory from GameState to FileSystem. --- .../GameEngine/Include/Common/FileSystem.h | 1 + .../GameEngine/Include/Common/GameState.h | 2 - .../Source/Common/System/FileSystem.cpp | 43 +++++++++++++++++++ .../Common/System/SaveGame/GameState.cpp | 42 +----------------- .../GameEngine/Include/Common/FileSystem.h | 1 + .../GameEngine/Include/Common/GameState.h | 2 - .../Source/Common/System/FileSystem.cpp | 43 +++++++++++++++++++ .../Common/System/SaveGame/GameState.cpp | 42 +----------------- 8 files changed, 92 insertions(+), 84 deletions(-) diff --git a/Generals/Code/GameEngine/Include/Common/FileSystem.h b/Generals/Code/GameEngine/Include/Common/FileSystem.h index a742788a6c4..cac342148e3 100644 --- a/Generals/Code/GameEngine/Include/Common/FileSystem.h +++ b/Generals/Code/GameEngine/Include/Common/FileSystem.h @@ -137,6 +137,7 @@ class FileSystem : public SubsystemInterface void loadMusicFilesFromCD(); void unloadMusicFilesFromCD(); AsciiString normalizePath(const AsciiString& path) const; ///< normalizes a file path. The path can refer to a directory. File path must be absolute, but does not need to exist. Returns an empty string on failure. + static Bool isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath); ///< determines if a file path is within a base path. Both paths must be absolute, but do not need to exist. protected: diff --git a/Generals/Code/GameEngine/Include/Common/GameState.h b/Generals/Code/GameEngine/Include/Common/GameState.h index ea1979d04d5..4d0a5a423cc 100644 --- a/Generals/Code/GameEngine/Include/Common/GameState.h +++ b/Generals/Code/GameEngine/Include/Common/GameState.h @@ -195,8 +195,6 @@ class GameState : public SubsystemInterface, AsciiString getMapLeafName(const AsciiString& in) const; - Bool isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const; - protected: // snapshot methods diff --git a/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp b/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp index 49480d9ea69..65e31318951 100644 --- a/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/FileSystem.cpp @@ -329,4 +329,47 @@ void FileSystem::unloadMusicFilesFromCD() AsciiString FileSystem::normalizePath(const AsciiString& path) const { return TheLocalFileSystem->normalizePath(path); +} + +//============================================================================ +// FileSystem::isPathInDirectory +//============================================================================ +Bool FileSystem::isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) +{ + AsciiString testPathNormalized = TheFileSystem->normalizePath(testPath); + AsciiString basePathNormalized = TheFileSystem->normalizePath(basePath); + + if (basePathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize base directory path '%s'.\n", basePath.str())); + return false; + } + else if (testPathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize file path '%s'.\n", testPath.str())); + return false; + } + +#ifdef _WIN32 + const char* pathSep = "\\"; +#else + const char* pathSep = "/"; +#endif + + if (!basePathNormalized.endsWith(pathSep)) + { + basePathNormalized.concat(pathSep); + } + +#ifdef _WIN32 + if (!testPathNormalized.startsWithNoCase(basePathNormalized)) +#else + if (!testPathNormalized.startsWith(basePathNormalized)) +#endif + { + DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').\n", testPath.str(), testPathNormalized.str(), basePath.str(), basePathNormalized.str())); + return false; + } + + return true; } \ No newline at end of file diff --git a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index e0016b988ec..8799456a3bb 100644 --- a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -783,7 +783,7 @@ AsciiString GameState::getFilePathInSaveDirectory(const AsciiString& leaf) const //------------------------------------------------------------------------------------------------- Bool GameState::isInSaveDirectory(const AsciiString& path) const { - return isPathInDirectory(path, getSaveDirectory()); + return FileSystem::isPathInDirectory(path, getSaveDirectory()); } // ------------------------------------------------------------------------------------------------ @@ -935,7 +935,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const return AsciiString::TheEmptyString; } - if (!isPathInDirectory(prefix, containingBasePath)) + if (!FileSystem::isPathInDirectory(prefix, containingBasePath)) { return AsciiString::TheEmptyString; } @@ -944,44 +944,6 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const return prefix; } -Bool GameState::isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const -{ - AsciiString filePathNormalized = TheFileSystem->normalizePath(testPath); - AsciiString basePathNormalized = TheFileSystem->normalizePath(basePath); - - if (basePathNormalized.isEmpty()) - { - DEBUG_CRASH(("Unable to normalize base directory path '%s'.\n", basePath.str())); - return false; - } - else if (filePathNormalized.isEmpty()) - { - DEBUG_CRASH(("Unable to normalize file path '%s'.\n", testPath.str())); - return false; - } -#ifdef _WIN32 - if (!basePathNormalized.endsWith("\\")) - { - basePathNormalized.concat("\\"); - } - - if (!filePathNormalized.startsWithNoCase(basePathNormalized)) -#else - if (!basePathNormalized.endsWith("/")) - { - basePathNormalized.concat("/"); - } - - if (!filePathNormalized.startsWith(basePathNormalized)) -#endif - { - DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').\n", testPath.str(), filePathNormalized.str(), basePath.str(), basePathNormalized.str())); - return false; - } - - return true; -} - // ------------------------------------------------------------------------------------------------ /** Does the save game file exist */ // ------------------------------------------------------------------------------------------------ diff --git a/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h b/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h index 05a6d6e7975..755e1c76b98 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/FileSystem.h @@ -141,6 +141,7 @@ class FileSystem : public SubsystemInterface void loadMusicFilesFromCD(); void unloadMusicFilesFromCD(); AsciiString normalizePath(const AsciiString& path) const; ///< normalizes a file path. The path can refer to a directory. File path must be absolute, but does not need to exist. Returns an empty string on failure. + static Bool isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath); ///< determines if a file path is within a base path. Both paths must be absolute, but do not need to exist. protected: mutable std::map m_fileExist; }; diff --git a/GeneralsMD/Code/GameEngine/Include/Common/GameState.h b/GeneralsMD/Code/GameEngine/Include/Common/GameState.h index 4e945c332ab..16a098c04ed 100644 --- a/GeneralsMD/Code/GameEngine/Include/Common/GameState.h +++ b/GeneralsMD/Code/GameEngine/Include/Common/GameState.h @@ -195,8 +195,6 @@ class GameState : public SubsystemInterface, AsciiString getMapLeafName(const AsciiString& in) const; - Bool isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const; - protected: // snapshot methods diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp index b08d79ee98c..4f8a7d63b3f 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/FileSystem.cpp @@ -346,4 +346,47 @@ void FileSystem::unloadMusicFilesFromCD() AsciiString FileSystem::normalizePath(const AsciiString& path) const { return TheLocalFileSystem->normalizePath(path); +} + +//============================================================================ +// FileSystem::isPathInDirectory +//============================================================================ +Bool FileSystem::isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) +{ + AsciiString testPathNormalized = TheFileSystem->normalizePath(testPath); + AsciiString basePathNormalized = TheFileSystem->normalizePath(basePath); + + if (basePathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize base directory path '%s'.\n", basePath.str())); + return false; + } + else if (testPathNormalized.isEmpty()) + { + DEBUG_CRASH(("Unable to normalize file path '%s'.\n", testPath.str())); + return false; + } + +#ifdef _WIN32 + const char* pathSep = "\\"; +#else + const char* pathSep = "/"; +#endif + + if (!basePathNormalized.endsWith(pathSep)) + { + basePathNormalized.concat(pathSep); + } + +#ifdef _WIN32 + if (!testPathNormalized.startsWithNoCase(basePathNormalized)) +#else + if (!testPathNormalized.startsWith(basePathNormalized)) +#endif + { + DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').\n", testPath.str(), testPathNormalized.str(), basePath.str(), basePathNormalized.str())); + return false; + } + + return true; } \ No newline at end of file diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index 20e5301e619..1fe48dd3d1d 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -783,7 +783,7 @@ AsciiString GameState::getFilePathInSaveDirectory(const AsciiString& leaf) const //------------------------------------------------------------------------------------------------- Bool GameState::isInSaveDirectory(const AsciiString& path) const { - return isPathInDirectory(path, getSaveDirectory()); + return FileSystem::isPathInDirectory(path, getSaveDirectory()); } // ------------------------------------------------------------------------------------------------ @@ -935,7 +935,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const return AsciiString::TheEmptyString; } - if (!isPathInDirectory(prefix, containingBasePath)) + if (!FileSystem::isPathInDirectory(prefix, containingBasePath)) { return AsciiString::TheEmptyString; } @@ -944,44 +944,6 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const return prefix; } -Bool GameState::isPathInDirectory(const AsciiString& testPath, const AsciiString& basePath) const -{ - AsciiString filePathNormalized = TheFileSystem->normalizePath(testPath); - AsciiString basePathNormalized = TheFileSystem->normalizePath(basePath); - - if (basePathNormalized.isEmpty()) - { - DEBUG_CRASH(("Unable to normalize base directory path '%s'.\n", basePath.str())); - return false; - } - else if (filePathNormalized.isEmpty()) - { - DEBUG_CRASH(("Unable to normalize file path '%s'.\n", testPath.str())); - return false; - } -#ifdef _WIN32 - if (!basePathNormalized.endsWith("\\")) - { - basePathNormalized.concat("\\"); - } - - if (!filePathNormalized.startsWithNoCase(basePathNormalized)) -#else - if (!basePathNormalized.endsWith("/")) - { - basePathNormalized.concat("/"); - } - - if (!filePathNormalized.startsWith(basePathNormalized)) -#endif - { - DEBUG_CRASH(("Normalized file path for '%s': '%s' was outside the expected base path of '%s' (normalized: '%s').\n", testPath.str(), filePathNormalized.str(), basePath.str(), basePathNormalized.str())); - return false; - } - - return true; -} - // ------------------------------------------------------------------------------------------------ /** Does the save game file exist */ // ------------------------------------------------------------------------------------------------ From a805a44a2c0c02e46dfd1e4b725a3e8671a1d9cb Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Tue, 17 Jun 2025 22:25:25 +0200 Subject: [PATCH 6/9] Determine required buffer size for normalized path at run-time. --- .../Win32Device/Common/Win32LocalFileSystem.cpp | 16 ++++++++++++---- .../Win32Device/Common/Win32LocalFileSystem.cpp | 16 ++++++++++++---- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp b/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp index 0e517be5e5b..19b84ef57d4 100644 --- a/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp +++ b/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp @@ -217,13 +217,21 @@ Bool Win32LocalFileSystem::createDirectory(AsciiString directory) AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) const { - TCHAR normalizeBuffer[MAX_PATH]; - DWORD charsWritten = GetFullPathName(filePath.str(), MAX_PATH, normalizeBuffer, NULL); - if (charsWritten == 0 || charsWritten > MAX_PATH) + DWORD retval = GetFullPathNameA(filePath.str(), 0, NULL, NULL); + if (retval == 0) { + DEBUG_LOG(("Unable to determine buffer size for normalized file path. Error=(%d).\n", GetLastError())); // empty string is used as a non-success return AsciiString::TheEmptyString; } - return AsciiString(normalizeBuffer); + AsciiString normalizedFilePath; + retval = GetFullPathNameA(filePath.str(), retval, normalizedFilePath.getBufferForRead(retval - 1), NULL); + if (retval == 0) + { + DEBUG_LOG(("Unable to normalize file path '%s'. Error=(%d).\n", filePath.str(), GetLastError())); + return AsciiString::TheEmptyString; + } + + return normalizedFilePath; } \ No newline at end of file diff --git a/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp b/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp index 37e4f523502..41fd959ed45 100644 --- a/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp +++ b/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp @@ -217,13 +217,21 @@ Bool Win32LocalFileSystem::createDirectory(AsciiString directory) AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) const { - TCHAR normalizeBuffer[MAX_PATH]; - DWORD charsWritten = GetFullPathName(filePath.str(), MAX_PATH, normalizeBuffer, NULL); - if (charsWritten == 0 || charsWritten > MAX_PATH) + DWORD retval = GetFullPathNameA(filePath.str(), 0, NULL, NULL); + if (retval == 0) { + DEBUG_LOG(("Unable to determine buffer size for normalized file path. Error=(%d).\n", GetLastError())); // empty string is used as a non-success return AsciiString::TheEmptyString; } - return AsciiString(normalizeBuffer); + AsciiString normalizedFilePath; + retval = GetFullPathNameA(filePath.str(), retval, normalizedFilePath.getBufferForRead(retval - 1), NULL); + if (retval == 0) + { + DEBUG_LOG(("Unable to normalize file path '%s'. Error=(%d).\n", filePath.str(), GetLastError())); + return AsciiString::TheEmptyString; + } + + return normalizedFilePath; } From ab7adc61af67d4e96a346aa398d396bb5120e527 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Wed, 18 Jun 2025 09:37:59 +0200 Subject: [PATCH 7/9] Update comments. --- .../GameEngine/Source/GameNetwork/ConnectionManager.cpp | 7 ++++--- Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp | 3 +++ .../GameEngine/Source/GameNetwork/ConnectionManager.cpp | 7 ++++--- GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp | 3 +++ .../Source/StdDevice/Common/StdLocalFileSystem.cpp | 2 +- 5 files changed, 15 insertions(+), 7 deletions(-) diff --git a/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp b/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp index ce6714215e9..b2df6bad2af 100644 --- a/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp +++ b/Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp @@ -689,11 +689,12 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg) #endif AsciiString realFileName = msg->getRealFilename(); - DEBUG_ASSERTLOG((!realFileName.isEmpty(), "Got a file name transferred that failed to normalize: '%s'!\n", msg->getPortableFilename().str())); - if (realFileName.isEmpty()) { - // As the file name failed to normalize (in other words is bogus), do nothing and let the transfer time out. + // TheSuperHackers @security slurmlord 18/06/2025 As the file name/path from the NetFileCommandMsg failed to normalize, + // in other words is bogus and points outside of the approved target directory, avoid an arbitrary file overwrite vulnerability + // by simply returning and let the transfer time out. + DEBUG_LOG(("Got a file name transferred that failed to normalize: '%s'!\n", msg->getPortableFilename().str())); return; } diff --git a/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index a9f3096c74e..16e4f70c9ec 100644 --- a/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -1047,6 +1047,9 @@ Bool ParseAsciiStringToGameInfo(GameInfo *game, AsciiString options) AsciiString realMapName = TheGameState->portableMapPathToRealMapPath(mapName); if (realMapName.isEmpty()) { + // TheSuperHackers @security slurmlord 18/06/2025 As the map file name/path from the AsciiString failed to normalize, + // in other words is bogus and points outside of the approved target directory for maps, avoid an arbitrary file overwrite vulnerability + // (if the save or network game embeds a custom map to store at the location) by flagging the options as not OK and rejecting the game. optionsOk = FALSE; DEBUG_LOG(("ParseAsciiStringToGameInfo - saw bogus map name ('%s'); quitting\n", mapName.str())); break; diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp index 302deceb7d5..9900f2e2c56 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp @@ -689,11 +689,12 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg) #endif AsciiString realFileName = msg->getRealFilename(); - DEBUG_ASSERTLOG((!realFileName.isEmpty(), "Got a file name transferred that failed to normalize: '%s'!\n", msg->getPortableFilename().str())); - if (realFileName.isEmpty()) { - // As the file name failed to normalize (in other words is bogus), do nothing and let the transfer time out. + // TheSuperHackers @security slurmlord 18/06/2025 As the file name/path from the NetFileCommandMsg failed to normalize, + // in other words is bogus and points outside of the approved target directory, avoid an arbitrary file overwrite vulnerability + // by simply returning and let the transfer time out. + DEBUG_LOG(("Got a file name transferred that failed to normalize: '%s'!\n", msg->getPortableFilename().str())); return; } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 5d7f230f122..2335328d091 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -1084,6 +1084,9 @@ Bool ParseAsciiStringToGameInfo(GameInfo *game, AsciiString options) AsciiString realMapName = TheGameState->portableMapPathToRealMapPath(mapName); if (realMapName.isEmpty()) { + // TheSuperHackers @security slurmlord 18/06/2025 As the map file name/path from the AsciiString failed to normalize, + // in other words is bogus and points outside of the approved target directory for maps, avoid an arbitrary file overwrite vulnerability + // (if the save or network game embeds a custom map to store at the location) by flagging the options as not OK and rejecting the game. optionsOk = FALSE; DEBUG_LOG(("ParseAsciiStringToGameInfo - saw bogus map name ('%s'); quitting\n", mapName.str())); break; diff --git a/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp b/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp index f815f5abfa3..5b21112b064 100644 --- a/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp +++ b/GeneralsMD/Code/GameEngineDevice/Source/StdDevice/Common/StdLocalFileSystem.cpp @@ -344,7 +344,7 @@ AsciiString StdLocalFileSystem::normalizePath(const AsciiString& filePath) const { std::string nonNormalized(filePath.str()); #ifndef _WIN32 - // Replace backslashes with forward slashes on unix + // Replace backslashes with forward slashes on non-Windows platforms std::replace(unNormalized.begin(), unNormalized.end(), '\\', '/'); #endif std::filesystem::path pathNonNormalized(nonNormalized); From 68f668eb7987d79cca7b40be1598390e8385f985 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Wed, 18 Jun 2025 21:40:26 +0200 Subject: [PATCH 8/9] Update comments and debug strings from code review. --- .../GameEngine/Source/Common/System/SaveGame/GameState.cpp | 2 -- Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp | 2 +- .../Source/Win32Device/Common/Win32LocalFileSystem.cpp | 5 ++--- .../GameEngine/Source/Common/System/SaveGame/GameState.cpp | 2 -- GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp | 2 +- .../Source/Win32Device/Common/Win32LocalFileSystem.cpp | 5 ++--- 6 files changed, 6 insertions(+), 12 deletions(-) diff --git a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index 8799456a3bb..acc5f06788e 100644 --- a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -930,8 +930,6 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const else { DEBUG_CRASH(("Map file was not found in any of the expected directories; this is impossible")); - //throw INI_INVALID_DATA; - // uncaught exceptions crash us. better to just use a bad path. return AsciiString::TheEmptyString; } diff --git a/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 16e4f70c9ec..0d564421d8b 100644 --- a/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/Generals/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -1049,7 +1049,7 @@ Bool ParseAsciiStringToGameInfo(GameInfo *game, AsciiString options) { // TheSuperHackers @security slurmlord 18/06/2025 As the map file name/path from the AsciiString failed to normalize, // in other words is bogus and points outside of the approved target directory for maps, avoid an arbitrary file overwrite vulnerability - // (if the save or network game embeds a custom map to store at the location) by flagging the options as not OK and rejecting the game. + // if the save or network game embeds a custom map to store at the location, by flagging the options as not OK and rejecting the game. optionsOk = FALSE; DEBUG_LOG(("ParseAsciiStringToGameInfo - saw bogus map name ('%s'); quitting\n", mapName.str())); break; diff --git a/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp b/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp index 19b84ef57d4..c768553cfff 100644 --- a/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp +++ b/Generals/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp @@ -220,8 +220,7 @@ AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) con DWORD retval = GetFullPathNameA(filePath.str(), 0, NULL, NULL); if (retval == 0) { - DEBUG_LOG(("Unable to determine buffer size for normalized file path. Error=(%d).\n", GetLastError())); - // empty string is used as a non-success + DEBUG_LOG(("Unable to determine buffer size for normalized file path. Error=(%u).\n", GetLastError())); return AsciiString::TheEmptyString; } @@ -229,7 +228,7 @@ AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) con retval = GetFullPathNameA(filePath.str(), retval, normalizedFilePath.getBufferForRead(retval - 1), NULL); if (retval == 0) { - DEBUG_LOG(("Unable to normalize file path '%s'. Error=(%d).\n", filePath.str(), GetLastError())); + DEBUG_LOG(("Unable to normalize file path '%s'. Error=(%u).\n", filePath.str(), GetLastError())); return AsciiString::TheEmptyString; } diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index 1fe48dd3d1d..7a834a69d7d 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -930,8 +930,6 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const else { DEBUG_CRASH(("Map file was not found in any of the expected directories; this is impossible")); - //throw INI_INVALID_DATA; - // uncaught exceptions crash us. better to just use a bad path. return AsciiString::TheEmptyString; } diff --git a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp index 2335328d091..8d6b22755f5 100644 --- a/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp +++ b/GeneralsMD/Code/GameEngine/Source/GameNetwork/GameInfo.cpp @@ -1086,7 +1086,7 @@ Bool ParseAsciiStringToGameInfo(GameInfo *game, AsciiString options) { // TheSuperHackers @security slurmlord 18/06/2025 As the map file name/path from the AsciiString failed to normalize, // in other words is bogus and points outside of the approved target directory for maps, avoid an arbitrary file overwrite vulnerability - // (if the save or network game embeds a custom map to store at the location) by flagging the options as not OK and rejecting the game. + // if the save or network game embeds a custom map to store at the location, by flagging the options as not OK and rejecting the game. optionsOk = FALSE; DEBUG_LOG(("ParseAsciiStringToGameInfo - saw bogus map name ('%s'); quitting\n", mapName.str())); break; diff --git a/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp b/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp index 41fd959ed45..5595d538273 100644 --- a/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp +++ b/GeneralsMD/Code/GameEngineDevice/Source/Win32Device/Common/Win32LocalFileSystem.cpp @@ -220,8 +220,7 @@ AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) con DWORD retval = GetFullPathNameA(filePath.str(), 0, NULL, NULL); if (retval == 0) { - DEBUG_LOG(("Unable to determine buffer size for normalized file path. Error=(%d).\n", GetLastError())); - // empty string is used as a non-success + DEBUG_LOG(("Unable to determine buffer size for normalized file path. Error=(%u).\n", GetLastError())); return AsciiString::TheEmptyString; } @@ -229,7 +228,7 @@ AsciiString Win32LocalFileSystem::normalizePath(const AsciiString& filePath) con retval = GetFullPathNameA(filePath.str(), retval, normalizedFilePath.getBufferForRead(retval - 1), NULL); if (retval == 0) { - DEBUG_LOG(("Unable to normalize file path '%s'. Error=(%d).\n", filePath.str(), GetLastError())); + DEBUG_LOG(("Unable to normalize file path '%s'. Error=(%u).\n", filePath.str(), GetLastError())); return AsciiString::TheEmptyString; } From efc4d98549358a26811de0e6acf20dbd05dee591 Mon Sep 17 00:00:00 2001 From: Slurmlord Date: Wed, 18 Jun 2025 21:46:59 +0200 Subject: [PATCH 9/9] Comment on why an empty string is returned. --- .../Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp | 1 + .../Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index acc5f06788e..4922a96bbd0 100644 --- a/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -930,6 +930,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const else { DEBUG_CRASH(("Map file was not found in any of the expected directories; this is impossible")); + // Empty string represents a failure, either caused by an invalid prefix or a relative path leading outside the base path return AsciiString::TheEmptyString; } diff --git a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp index 7a834a69d7d..8444bfc9dd0 100644 --- a/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp +++ b/GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp @@ -930,6 +930,7 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const else { DEBUG_CRASH(("Map file was not found in any of the expected directories; this is impossible")); + // Empty string represents a failure, either caused by an invalid prefix or a relative path leading outside the base path. return AsciiString::TheEmptyString; }