Skip to content

Commit 5648a6d

Browse files
committed
refactor(security): address code review feedback on extension validation
1 parent 28c81c1 commit 5648a6d

File tree

4 files changed

+0
-59
lines changed

4 files changed

+0
-59
lines changed

Core/GameEngine/Include/Common/FileSystem.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,6 @@ 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
165164

166165
protected:
167166
#if ENABLE_FILESYSTEM_EXISTENCE_CACHE

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

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -457,47 +457,3 @@ 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-
}

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -933,13 +933,6 @@ 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-
943936
prefix.toLower();
944937
return prefix;
945938
}

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

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -933,13 +933,6 @@ 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-
943936
prefix.toLower();
944937
return prefix;
945938
}

0 commit comments

Comments
 (0)