Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
97d61d7
fix(security): add file content validation for map transfer
bobtista Nov 6, 2025
73e763d
refactor(security): add STRICMP macro for cross-platform string compa…
bobtista Dec 17, 2025
0599333
refactor(security): split validation into helper function with early …
bobtista Dec 17, 2025
c2d0135
refactor(security): simplify .str/.txt extension handling logic
bobtista Dec 17, 2025
58ae26c
fix(security): reduce file size limits to reasonable values
bobtista Dec 17, 2025
59f93ca
docs(security): add comment explaining transfer abort behavior on val…
bobtista Dec 17, 2025
41048c3
feat(network): Validate transfer file content in memory before writin…
bobtista Feb 12, 2026
c87d395
style(security): Replace NULL with nullptr in transfer file validation
bobtista Feb 12, 2026
1812801
style(security): Use std::min for INI null-byte check byte count
bobtista Feb 16, 2026
fc9a68c
style(security): Use memcmp for map file magic bytes check
bobtista Feb 16, 2026
2bdc12a
feat(security): Add TGA 2.0 footer signature validation for transferr…
bobtista Feb 16, 2026
4505e7b
refactor(security): Extract TransferFileRule into anonymous namespace…
bobtista Feb 16, 2026
bc37447
fix(security): Require minimum 44 bytes for TGA validation (18 header…
bobtista Feb 16, 2026
f846100
fix(security): Use UnsignedInt for dataSize and maxSize in transfer f…
bobtista Feb 17, 2026
9e25dda
style(security): Rename TransferFileType enum values to PascalCase
bobtista Feb 17, 2026
237cf08
refactor(security): Use enum as array index for transfer file rules
bobtista Feb 17, 2026
ce419b8
style(security): Add consistent braces to switch cases in transfer va…
bobtista Feb 17, 2026
21f2594
refactor(security): Use TGA2Footer struct for TGA footer validation
bobtista Feb 17, 2026
94fc33c
fix(security): Scan entire INI file for null bytes instead of first 5…
bobtista Feb 17, 2026
08b64fd
style(security): Use TGA2_SIGNATURE macro instead of hardcoded string
bobtista Feb 17, 2026
71c7464
style(security): Rename lastDot to fileExt for clarity
bobtista Feb 19, 2026
63312bc
refactor(security): Add explicit TransferFileType_Invalid sentinel value
bobtista Feb 19, 2026
871abfb
style(security): Extract isTGA2 boolean for TGA footer validation rea…
bobtista Feb 19, 2026
44680b4
refactor(security): Move transfer file validation from FileSystem to …
bobtista Feb 24, 2026
f73fe40
nit: remove extra newline
bobtista Feb 24, 2026
bf1adbd
nit: remove redundant comment
bobtista Feb 24, 2026
7a0f5a5
tweak: adjust transfer file size limits based on community map data
bobtista Apr 3, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Core/GameEngine/Include/Common/FileSystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ class FileSystem : public SubsystemInterface

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.
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.
static Bool hasValidTransferFileContent(const AsciiString& filePath); ///< validates that a file's content matches expected format for its extension during map transfer operations. Checks file headers, magic bytes, size limits, and basic structure. TheSuperHackers @security bobtista 06/11/2025

protected:
#if ENABLE_FILESYSTEM_EXISTENCE_CACHE
Expand Down
128 changes: 128 additions & 0 deletions Core/GameEngine/Source/Common/System/FileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@
#include "Common/file.h"
#include "Common/FileSystem.h"

#ifdef _WIN32
#define STRICMP(s1, s2) _stricmp(s1, s2)
#else
#define STRICMP(s1, s2) strcasecmp(s1, s2)
#endif

Comment thread
Skyaero42 marked this conversation as resolved.
Outdated
#include "Common/ArchiveFileSystem.h"
#include "Common/CDManager.h"
#include "Common/GameAudio.h"
Expand Down Expand Up @@ -457,3 +463,125 @@ Bool FileSystem::isPathInDirectory(const AsciiString& testPath, const AsciiStrin

return true;
}

Comment thread
bobtista marked this conversation as resolved.
Outdated
//============================================================================
// FileSystem::hasValidTransferFileContent
//============================================================================
// TheSuperHackers @security bobtista 06/11/2025
// Validates file content format during map transfer operations.
// Checks file headers, magic bytes, size limits, and basic structure to prevent
// malformed or malicious files from being processed.
//============================================================================
static Bool validateFileContent(File* file, const AsciiString& filePath, const char* extension)
Comment thread
Skyaero42 marked this conversation as resolved.
Outdated
{
const Int fileSize = file->size();

const Int MAX_MAP_SIZE = 512 * 1024;
const Int MAX_INI_SIZE = 512 * 1024;
const Int MAX_STR_SIZE = 512 * 1024;
const Int MAX_TGA_SIZE = 2 * 1024 * 1024;
const Int MAX_TXT_SIZE = 512 * 1024;
const Int MAX_WAK_SIZE = 512 * 1024;

if (STRICMP(extension, ".map") == 0)
{
if (fileSize > MAX_MAP_SIZE)
{
DEBUG_LOG(("Map file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
Comment thread
Skyaero42 marked this conversation as resolved.
Outdated
return false;
}

UnsignedByte header[4];
file->read(header, 4);
if (header[0] != 'C' || header[1] != 'k' || header[2] != 'M' || header[3] != 'p')
{
DEBUG_LOG(("Map file '%s' has invalid magic bytes.", filePath.str()));
return false;
}
return true;
}

if (STRICMP(extension, ".ini") == 0)
{
if (fileSize > MAX_INI_SIZE)
{
DEBUG_LOG(("INI file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
return false;
}

UnsignedByte sample[512];
Int bytesToRead = fileSize < 512 ? fileSize : 512;
file->read(sample, bytesToRead);

for (Int i = 0; i < bytesToRead; ++i)
{
if (sample[i] == 0)
{
DEBUG_LOG(("INI file '%s' contains null bytes (likely binary).", filePath.str()));
return false;
}
}
return true;
}

Bool isStr = (STRICMP(extension, ".str") == 0);
Bool isTxt = (STRICMP(extension, ".txt") == 0);
if (isStr || isTxt)
{
Int maxSize = isTxt ? MAX_TXT_SIZE : MAX_STR_SIZE;
if (fileSize > maxSize)
{
DEBUG_LOG(("Text file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
return false;
}
return true;
}

if (STRICMP(extension, ".tga") == 0)
{
if (fileSize > MAX_TGA_SIZE)
{
DEBUG_LOG(("TGA file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
return false;
}
if (fileSize < 18)
{
DEBUG_LOG(("TGA file '%s' is too small to be valid (minimum 18 bytes).", filePath.str()));
return false;
}
return true;
}

if (STRICMP(extension, ".wak") == 0)
{
if (fileSize > MAX_WAK_SIZE)
{
DEBUG_LOG(("WAK file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
return false;
}
return true;
}

DEBUG_LOG(("File '%s' has unrecognized extension for content validation.", filePath.str()));
return false;
}

Bool FileSystem::hasValidTransferFileContent(const AsciiString& filePath)
{
const char* lastDot = strrchr(filePath.str(), '.');
if (lastDot == NULL)
{
return false;
}

File* file = TheLocalFileSystem->openFile(filePath.str(), File::READ | File::BINARY);
if (file == NULL)
{
DEBUG_LOG(("Cannot open file '%s' for content validation.", filePath.str()));
return false;
}

Bool isValid = validateFileContent(file, filePath, lastDot);
file->close();
return isValid;
}
16 changes: 16 additions & 0 deletions Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,22 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg)
fp = NULL;
DEBUG_LOG(("Wrote %d bytes to file %s!", len, realFileName.str()));

// TheSuperHackers @security bobtista 06/11/2025 Validate file content after writing
if (!FileSystem::hasValidTransferFileContent(realFileName))
Comment thread
xezon marked this conversation as resolved.
Outdated
{
DEBUG_LOG(("File '%s' failed content validation, deleting. Transfer aborted.", realFileName.str()));
remove(realFileName.str());
#ifdef COMPRESS_TARGAS
if (deleteBuf)
{
delete[] buf;
buf = NULL;
}
#endif
// Transfer is silently aborted - file is deleted and no progress message is sent.
// The sender will timeout waiting for progress confirmation.
return;
Comment thread
Skyaero42 marked this conversation as resolved.
Outdated
}
}
else
{
Expand Down
Loading