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

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 @@ -54,6 +54,7 @@
#include "Common/GameAudio.h"
#include "Common/LocalFileSystem.h"
#include "Common/PerfTimer.h"
#include "TARGA.h"


DECLARE_PERF_TIMER(FileSystem)
Expand Down Expand Up @@ -457,3 +458,130 @@ Bool FileSystem::isPathInDirectory(const AsciiString& testPath, const AsciiStrin

return true;
}

Comment thread
bobtista marked this conversation as resolved.
Outdated
namespace
{

enum TransferFileType
{
TransferFileType_Map,
TransferFileType_Ini,
TransferFileType_Str,
TransferFileType_Txt,
TransferFileType_Tga,
TransferFileType_Wak,
TransferFileType_Count
};

struct TransferFileRule
{
const char* ext;
UnsignedInt maxSize;
};

const TransferFileRule transferFileRules[TransferFileType_Count] =
{
{ ".map", 5 * 1024 * 1024 },
{ ".ini", 512 * 1024 },
{ ".str", 512 * 1024 },
{ ".txt", 512 * 1024 },
{ ".tga", 2 * 1024 * 1024 },
{ ".wak", 512 * 1024 },
};

TransferFileType getTransferFileType(const char* extension)
Comment thread
xezon marked this conversation as resolved.
Outdated
{
for (Int i = 0; i < TransferFileType_Count; ++i)
{
if (stricmp(extension, transferFileRules[i].ext) == 0)
{
return static_cast<TransferFileType>(i);
}
}
return TransferFileType_Count;
}

} // namespace

//============================================================================
// 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, UnsignedInt dataSize)
{
const char* lastDot = strrchr(filePath.str(), '.');
Comment thread
bobtista marked this conversation as resolved.
Outdated
if (lastDot == nullptr)
{
DEBUG_LOG(("File '%s' has no extension for content validation.", filePath.str()));
return false;
}

const TransferFileType fileType = getTransferFileType(lastDot);
if (fileType == TransferFileType_Count)
Comment thread
bobtista 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
const TransferFileRule& rule = transferFileRules[fileType];
if (dataSize > rule.maxSize)
{
DEBUG_LOG(("File '%s' exceeds maximum size (%u bytes, limit %u bytes).", filePath.str(), dataSize, rule.maxSize));
return false;
}

// Extension-specific content validation
switch (fileType)
{
case TransferFileType_Map:
{
if (dataSize < 4 || memcmp(data, "CkMp", 4) != 0)
Comment thread
bobtista marked this conversation as resolved.
Outdated
{
DEBUG_LOG(("Map file '%s' has invalid magic bytes.", filePath.str()));
return false;
}
break;
}

case TransferFileType_Ini:
{
for (UnsignedInt i = 0; i < dataSize; ++i)
{
if (data[i] == 0)
{
DEBUG_LOG(("INI file '%s' contains null bytes (likely binary).", filePath.str()));
return false;
}
}
break;
}

case TransferFileType_Tga:
{
if (dataSize < sizeof(TGAHeader) + sizeof(TGA2Footer))
{
DEBUG_LOG(("TGA file '%s' is too small to be valid.", filePath.str()));
return false;
}
TGA2Footer footer;
memcpy(&footer, data + dataSize - sizeof(footer), sizeof(footer));
if (memcmp(footer.Signature, TGA2_SIGNATURE, sizeof(footer.Signature)) != 0
|| footer.RsvdChar != '.'
|| footer.BZST != '\0')
Comment thread
bobtista marked this conversation as resolved.
Outdated
{
DEBUG_LOG(("TGA file '%s' is missing TRUEVISION-XFILE footer signature.", filePath.str()));
return false;
}
break;
}

default:
{
break;
}
}

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 = nullptr;
}
#endif // COMPRESS_TARGAS
return;
}

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