Skip to content

Commit 28c81c1

Browse files
committed
fix(security): add file extension validation for map transfer
1 parent d46feb9 commit 28c81c1

File tree

5 files changed

+96
-0
lines changed

5 files changed

+96
-0
lines changed

Core/GameEngine/Include/Common/FileSystem.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ class FileSystem : public SubsystemInterface
161161

162162
static AsciiString normalizePath(const AsciiString& path); ///< 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.
163163
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.
164+
static Bool hasValidTransferFileExtension(const AsciiString& filePath); ///< validates that a file path has an approved extension for map transfer operations. Returns true if extension is whitelisted (.map, .ini, .str, .wak, .tga, .txt), false otherwise. TheSuperHackers @security bobtista 06/11/2025
164165

165166
protected:
166167
#if ENABLE_FILESYSTEM_EXISTENCE_CACHE

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

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,3 +457,47 @@ Bool FileSystem::isPathInDirectory(const AsciiString& testPath, const AsciiStrin
457457

458458
return true;
459459
}
460+
461+
//============================================================================
462+
// FileSystem::hasValidTransferFileExtension
463+
//============================================================================
464+
// TheSuperHackers @security bobtista 06/11/2025
465+
// Validates file extensions for map transfer operations to prevent arbitrary
466+
// file types from being transferred over the network or loaded from save games.
467+
// This is a security measure to mitigate arbitrary file write vulnerabilities.
468+
//============================================================================
469+
Bool FileSystem::hasValidTransferFileExtension(const AsciiString& filePath)
470+
{
471+
static const char* validExtensions[] = {
472+
".map",
473+
".ini",
474+
".str",
475+
".wak",
476+
".tga",
477+
".txt"
478+
};
479+
static const Int numValidExtensions = sizeof(validExtensions) / sizeof(validExtensions[0]);
480+
481+
const char* lastDot = strrchr(filePath.str(), '.');
482+
483+
if (lastDot == NULL || lastDot[1] == '\0')
484+
{
485+
DEBUG_LOG(("File path '%s' has no extension.", filePath.str()));
486+
return false;
487+
}
488+
489+
for (Int i = 0; i < numValidExtensions; ++i)
490+
{
491+
#ifdef _WIN32
492+
if (_stricmp(lastDot, validExtensions[i]) == 0)
493+
#else
494+
if (strcasecmp(lastDot, validExtensions[i]) == 0)
495+
#endif
496+
{
497+
return true;
498+
}
499+
}
500+
501+
DEBUG_LOG(("File path '%s' has invalid extension '%s' for transfer operations.", filePath.str(), lastDot));
502+
return false;
503+
}

Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,36 @@
5353
#include "GameClient/DisconnectMenu.h"
5454
#include "GameClient/InGameUI.h"
5555

56+
static Bool hasValidTransferFileExtension(const AsciiString& filePath)
57+
{
58+
static const char* const validExtensions[] = {
59+
"map",
60+
"ini",
61+
"str",
62+
"wak",
63+
"tga",
64+
"txt"
65+
};
66+
67+
const char* fileExt = strrchr(filePath.str(), '.');
68+
69+
if (fileExt == NULL || fileExt[1] == '\0')
70+
{
71+
return false;
72+
}
73+
74+
fileExt++;
75+
76+
for (Int i = 0; i < ARRAY_SIZE(validExtensions); ++i)
77+
{
78+
if (stricmp(fileExt, validExtensions[i]) == 0)
79+
{
80+
return true;
81+
}
82+
}
83+
84+
return false;
85+
}
5686

5787
/**
5888
* Le destructor.
@@ -665,6 +695,13 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg)
665695
return;
666696
}
667697

698+
// TheSuperHackers @security bobtista 06/11/2025 Validate file extension to prevent arbitrary file types
699+
if (!hasValidTransferFileExtension(realFileName))
700+
{
701+
DEBUG_LOG(("File '%s' has invalid extension for transfer operations.", realFileName.str()));
702+
return;
703+
}
704+
668705
if (TheFileSystem->doesFileExist(realFileName.str()))
669706
{
670707
DEBUG_LOG(("File exists already!"));

Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,13 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const
933933
return AsciiString::TheEmptyString;
934934
}
935935

936+
// TheSuperHackers @security bobtista 06/11/2025 Validate file extension to prevent arbitrary file types
937+
if (!FileSystem::hasValidTransferFileExtension(prefix))
938+
{
939+
DEBUG_LOG(("File path '%s' has invalid extension for map transfer operations.", prefix.str()));
940+
return AsciiString::TheEmptyString;
941+
}
942+
936943
prefix.toLower();
937944
return prefix;
938945
}

GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,13 @@ AsciiString GameState::portableMapPathToRealMapPath(const AsciiString& in) const
933933
return AsciiString::TheEmptyString;
934934
}
935935

936+
// TheSuperHackers @security bobtista 06/11/2025 Validate file extension to prevent arbitrary file types
937+
if (!FileSystem::hasValidTransferFileExtension(prefix))
938+
{
939+
DEBUG_LOG(("File path '%s' has invalid extension for map transfer operations.", prefix.str()));
940+
return AsciiString::TheEmptyString;
941+
}
942+
936943
prefix.toLower();
937944
return prefix;
938945
}

0 commit comments

Comments
 (0)