Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 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, const UnsignedByte* data, Int dataSize); ///< validates transferred file content in memory before writing to disk.

protected:
#if ENABLE_FILESYSTEM_EXISTENCE_CACHE
Expand Down
90 changes: 90 additions & 0 deletions Core/GameEngine/Source/Common/System/FileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -457,3 +457,93 @@ Bool FileSystem::isPathInDirectory(const AsciiString& testPath, const AsciiStrin

return true;
}

Comment thread
bobtista marked this conversation as resolved.
Outdated
//============================================================================
// FileSystem::hasValidTransferFileContent
//============================================================================
// TheSuperHackers @security bobtista 12/02/2026 Validates transferred file
// content in memory before writing to disk.
Bool FileSystem::hasValidTransferFileContent(const AsciiString& filePath, const UnsignedByte* data, Int dataSize)
Comment thread
Caball009 marked this conversation as resolved.
Outdated
{
struct TransferFileRule
{
const char* ext;
Int maxSize;
};

static const TransferFileRule transferFileRules[] =
Comment thread
bobtista marked this conversation as resolved.
Outdated
{
{ ".map", 5 * 1024 * 1024 },
{ ".ini", 512 * 1024 },
{ ".str", 512 * 1024 },
{ ".txt", 512 * 1024 },
{ ".tga", 2 * 1024 * 1024 },
{ ".wak", 512 * 1024 },
};
Comment thread
bobtista marked this conversation as resolved.
Outdated

const char* lastDot = strrchr(filePath.str(), '.');
Comment thread
bobtista marked this conversation as resolved.
Outdated
if (lastDot == NULL)
Comment thread
xezon marked this conversation as resolved.
Outdated
{
DEBUG_LOG(("File '%s' has no extension for content validation.", filePath.str()));
return false;
}

// Find matching rule by extension
const TransferFileRule* matchedRule = NULL;
Comment thread
xezon marked this conversation as resolved.
Outdated
for (Int i = 0; i < ARRAY_SIZE(transferFileRules); ++i)
Comment thread
bobtista marked this conversation as resolved.
Outdated
{
if (stricmp(lastDot, transferFileRules[i].ext) == 0)
{
matchedRule = &transferFileRules[i];
break;
}
}

if (matchedRule == NULL)
Comment thread
xezon marked this conversation as resolved.
Outdated
{
DEBUG_LOG(("File '%s' has unrecognized extension '%s' for content validation.", filePath.str(), lastDot));
return false;
}

// Check size limit
if (dataSize > matchedRule->maxSize)
{
DEBUG_LOG(("File '%s' exceeds maximum size (%d bytes, limit %d bytes).", filePath.str(), dataSize, matchedRule->maxSize));
return false;
}

// Extension-specific content validation
if (stricmp(lastDot, ".map") == 0)
Comment thread
bobtista marked this conversation as resolved.
Outdated
{
// Validate magic bytes "CkMp"
if (dataSize < 4 || data[0] != 'C' || data[1] != 'k' || data[2] != 'M' || data[3] != 'p')
Comment thread
bobtista marked this conversation as resolved.
Outdated
{
DEBUG_LOG(("Map file '%s' has invalid magic bytes.", filePath.str()));
return false;
}
}
else if (stricmp(lastDot, ".ini") == 0)
{
// Check for null bytes to ensure text format
Int bytesToCheck = dataSize < 512 ? dataSize : 512;
Comment thread
xezon marked this conversation as resolved.
Outdated
for (Int i = 0; i < bytesToCheck; ++i)
{
if (data[i] == 0)
{
DEBUG_LOG(("INI file '%s' contains null bytes (likely binary).", filePath.str()));
return false;
}
}
}
else if (stricmp(lastDot, ".tga") == 0)
{
// Validate minimum TGA header size
Comment thread
bobtista marked this conversation as resolved.
Outdated
if (dataSize < 18)
{
DEBUG_LOG(("TGA file '%s' is too small to be valid (minimum 18 bytes).", filePath.str()));
return false;
Comment thread
bobtista marked this conversation as resolved.
Outdated
}
}

return true;
}
15 changes: 15 additions & 0 deletions Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "Common/CRCDebug.h"
#include "Common/Debug.h"
#include "Common/file.h"
#include "Common/FileSystem.h"
#include "Common/GameAudio.h"
#include "Common/LocalFileSystem.h"
#include "Common/Player.h"
Expand Down Expand Up @@ -734,6 +735,20 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg)
}
#endif // COMPRESS_TARGAS

// TheSuperHackers @security bobtista 12/02/2026 Validate file content in memory before writing to disk
if (!FileSystem::hasValidTransferFileContent(realFileName, buf, len))
Comment thread
xezon marked this conversation as resolved.
Outdated
{
DEBUG_LOG(("File '%s' failed content validation. Transfer aborted.", realFileName.str()));
#ifdef COMPRESS_TARGAS
if (deleteBuf)
{
delete[] buf;
buf = NULL;
Comment thread
xezon marked this conversation as resolved.
Outdated
}
#endif // COMPRESS_TARGAS
return;
}

File *fp = TheFileSystem->openFile(realFileName.str(), File::CREATE | File::BINARY | File::WRITE);
if (fp)
{
Expand Down
Loading