Skip to content

Commit 630dbcb

Browse files
bobtistabobtista
committed
fix(security): add file content validation for map transfer
1 parent 0bb57b2 commit 630dbcb

File tree

4 files changed

+200
-0
lines changed

4 files changed

+200
-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 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
164165

165166
protected:
166167
#if ENABLE_FILESYSTEM_EXISTENCE_CACHE

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

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

458458
return true;
459459
}
460+
461+
//============================================================================
462+
// FileSystem::hasValidTransferFileContent
463+
//============================================================================
464+
// TheSuperHackers @security bobtista 06/11/2025
465+
// Validates file content format during map transfer operations.
466+
// Checks file headers, magic bytes, size limits, and basic structure to prevent
467+
// malformed or malicious files from being processed.
468+
//============================================================================
469+
Bool FileSystem::hasValidTransferFileContent(const AsciiString& filePath)
470+
{
471+
File* file = TheLocalFileSystem->openFile(filePath.str(), File::READ | File::BINARY);
472+
if (file == NULL)
473+
{
474+
DEBUG_LOG(("Cannot open file '%s' for content validation.", filePath.str()));
475+
return false;
476+
}
477+
478+
const Int fileSize = file->size();
479+
Bool isValid = false;
480+
481+
const char* lastDot = strrchr(filePath.str(), '.');
482+
if (lastDot == NULL)
483+
{
484+
file->close();
485+
return false;
486+
}
487+
488+
const Int MAX_MAP_SIZE = 50 * 1024 * 1024;
489+
const Int MAX_INI_SIZE = 10 * 1024 * 1024;
490+
const Int MAX_STR_SIZE = 5 * 1024 * 1024;
491+
const Int MAX_TGA_SIZE = 20 * 1024 * 1024;
492+
const Int MAX_TXT_SIZE = 5 * 1024 * 1024;
493+
const Int MAX_WAK_SIZE = 10 * 1024 * 1024;
494+
495+
#ifdef _WIN32
496+
if (_stricmp(lastDot, ".map") == 0)
497+
#else
498+
if (strcasecmp(lastDot, ".map") == 0)
499+
#endif
500+
{
501+
if (fileSize > MAX_MAP_SIZE)
502+
{
503+
DEBUG_LOG(("Map file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
504+
isValid = false;
505+
}
506+
else
507+
{
508+
UnsignedByte header[4];
509+
file->read(header, 4);
510+
if (header[0] == 'C' && header[1] == 'k' && header[2] == 'M' && header[3] == 'p')
511+
{
512+
isValid = true;
513+
}
514+
else
515+
{
516+
DEBUG_LOG(("Map file '%s' has invalid magic bytes.", filePath.str()));
517+
isValid = false;
518+
}
519+
}
520+
}
521+
#ifdef _WIN32
522+
else if (_stricmp(lastDot, ".ini") == 0)
523+
#else
524+
else if (strcasecmp(lastDot, ".ini") == 0)
525+
#endif
526+
{
527+
if (fileSize > MAX_INI_SIZE)
528+
{
529+
DEBUG_LOG(("INI file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
530+
isValid = false;
531+
}
532+
else
533+
{
534+
UnsignedByte sample[512];
535+
Int bytesToRead = fileSize < 512 ? fileSize : 512;
536+
file->read(sample, bytesToRead);
537+
538+
Bool hasNullBytes = false;
539+
for (Int i = 0; i < bytesToRead; ++i)
540+
{
541+
if (sample[i] == 0)
542+
{
543+
hasNullBytes = true;
544+
break;
545+
}
546+
}
547+
548+
if (hasNullBytes)
549+
{
550+
DEBUG_LOG(("INI file '%s' contains null bytes (likely binary).", filePath.str()));
551+
isValid = false;
552+
}
553+
else
554+
{
555+
isValid = true;
556+
}
557+
}
558+
}
559+
#ifdef _WIN32
560+
else if (_stricmp(lastDot, ".str") == 0 || _stricmp(lastDot, ".txt") == 0)
561+
#else
562+
else if (strcasecmp(lastDot, ".str") == 0 || strcasecmp(lastDot, ".txt") == 0)
563+
#endif
564+
{
565+
Int maxSize = MAX_STR_SIZE;
566+
#ifdef _WIN32
567+
if (_stricmp(lastDot, ".txt") == 0)
568+
#else
569+
if (strcasecmp(lastDot, ".txt") == 0)
570+
#endif
571+
{
572+
maxSize = MAX_TXT_SIZE;
573+
}
574+
575+
if (fileSize > maxSize)
576+
{
577+
DEBUG_LOG(("Text file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
578+
isValid = false;
579+
}
580+
else
581+
{
582+
isValid = true;
583+
}
584+
}
585+
#ifdef _WIN32
586+
else if (_stricmp(lastDot, ".tga") == 0)
587+
#else
588+
else if (strcasecmp(lastDot, ".tga") == 0)
589+
#endif
590+
{
591+
if (fileSize > MAX_TGA_SIZE)
592+
{
593+
DEBUG_LOG(("TGA file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
594+
isValid = false;
595+
}
596+
else if (fileSize < 18)
597+
{
598+
DEBUG_LOG(("TGA file '%s' is too small to be valid (minimum 18 bytes).", filePath.str()));
599+
isValid = false;
600+
}
601+
else
602+
{
603+
isValid = true;
604+
}
605+
}
606+
#ifdef _WIN32
607+
else if (_stricmp(lastDot, ".wak") == 0)
608+
#else
609+
else if (strcasecmp(lastDot, ".wak") == 0)
610+
#endif
611+
{
612+
if (fileSize > MAX_WAK_SIZE)
613+
{
614+
DEBUG_LOG(("WAK file '%s' exceeds maximum size (%d bytes).", filePath.str(), fileSize));
615+
isValid = false;
616+
}
617+
else
618+
{
619+
isValid = true;
620+
}
621+
}
622+
else
623+
{
624+
DEBUG_LOG(("File '%s' has unrecognized extension for content validation.", filePath.str()));
625+
isValid = false;
626+
}
627+
628+
file->close();
629+
return isValid;
630+
}

‎Generals/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,20 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg)
705705
fp = NULL;
706706
DEBUG_LOG(("Wrote %d bytes to file %s!", len, realFileName.str()));
707707

708+
// TheSuperHackers @security bobtista 06/11/2025 Validate file content after writing
709+
if (!FileSystem::hasValidTransferFileContent(realFileName))
710+
{
711+
DEBUG_LOG(("File '%s' failed content validation, deleting.", realFileName.str()));
712+
TheLocalFileSystem->deleteFile(realFileName.str());
713+
#ifdef COMPRESS_TARGAS
714+
if (deleteBuf)
715+
{
716+
delete[] buf;
717+
buf = NULL;
718+
}
719+
#endif
720+
return;
721+
}
708722
}
709723
else
710724
{

‎GeneralsMD/Code/GameEngine/Source/GameNetwork/ConnectionManager.cpp‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -705,6 +705,20 @@ void ConnectionManager::processFile(NetFileCommandMsg *msg)
705705
fp = NULL;
706706
DEBUG_LOG(("Wrote %d bytes to file %s!", len, realFileName.str()));
707707

708+
// TheSuperHackers @security bobtista 06/11/2025 Validate file content after writing
709+
if (!FileSystem::hasValidTransferFileContent(realFileName))
710+
{
711+
DEBUG_LOG(("File '%s' failed content validation, deleting.", realFileName.str()));
712+
TheLocalFileSystem->deleteFile(realFileName.str());
713+
#ifdef COMPRESS_TARGAS
714+
if (deleteBuf)
715+
{
716+
delete[] buf;
717+
buf = NULL;
718+
}
719+
#endif
720+
return;
721+
}
708722
}
709723
else
710724
{

0 commit comments

Comments
 (0)