Skip to content

Commit 8727e1e

Browse files
stephanmeestersstephanmeesters
authored
fix: Replace sprintf with snprintf to prevent potential stack overflows (TheSuperHackers#2262)
1 parent a53adf2 commit 8727e1e

30 files changed

Lines changed: 112 additions & 96 deletions

File tree

‎Core/GameEngine/Source/Common/INI/INI.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,8 @@ UnsignedInt INI::load( AsciiString filename, INILoadType loadType, Xfer *pXfer )
425425
} catch (...) {
426426
DEBUG_CRASH(("Error parsing block '%s' in INI file '%s'", token, m_filename.str()) );
427427
char buff[1024];
428-
sprintf(buff, "Error parsing INI file '%s' (Line: '%s')\n", m_filename.str(), currentLine.str());
428+
snprintf(buff, ARRAY_SIZE(buff), "Error parsing INI file '%s' (Line: '%s')\n",
429+
m_filename.str(), currentLine.str());
429430

430431
throw INIException(buff);
431432
}
@@ -1564,7 +1565,8 @@ void INI::initFromINIMulti( void *what, const MultiIniFieldParse& parseTableList
15641565

15651566

15661567
char buff[1024];
1567-
sprintf(buff, "[LINE: %d - FILE: '%s'] Error reading field '%s'\n", INI::getLineNum(), INI::getFilename().str(), field);
1568+
snprintf(buff, ARRAY_SIZE(buff), "[LINE: %d - FILE: '%s'] Error reading field '%s'\n",
1569+
INI::getLineNum(), INI::getFilename().str(), field);
15681570
throw INIException(buff);
15691571
}
15701572

‎Core/GameEngine/Source/GameNetwork/GameSpy/StagingRoomGameInfo.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -861,7 +861,8 @@ void GameSpyStagingRoom::launchGame( void )
861861
req.buddyRequestType = BuddyRequest::BUDDYREQUEST_SETSTATUS;
862862
req.arg.status.status = GP_PLAYING;
863863
strcpy(req.arg.status.statusString, "Loading");
864-
sprintf(req.arg.status.locationString, "%s", WideCharStringToMultiByte(getGameName().str()).c_str());
864+
strlcpy(req.arg.status.locationString, WideCharStringToMultiByte(getGameName().str()).c_str(),
865+
ARRAY_SIZE(req.arg.status.locationString));
865866
TheGameSpyBuddyMessageQueue->addRequest(req);
866867

867868
delete TheNAT;

‎Core/GameEngine/Source/GameNetwork/GameSpy/Thread/PeerThread.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -670,22 +670,22 @@ static void updateBuddyStatus( GameSpyBuddyStatus status, Int groupRoom = 0, std
670670
case BUDDY_LOBBY:
671671
req.arg.status.status = GP_CHATTING;
672672
strcpy(req.arg.status.statusString, "Chatting");
673-
sprintf(req.arg.status.locationString, "%d", groupRoom);
673+
snprintf(req.arg.status.locationString, ARRAY_SIZE(req.arg.status.locationString), "%d", groupRoom);
674674
break;
675675
case BUDDY_STAGING:
676676
req.arg.status.status = GP_STAGING;
677677
strcpy(req.arg.status.statusString, "Staging");
678-
sprintf(req.arg.status.locationString, "%s", gameName.c_str());
678+
strlcpy(req.arg.status.locationString, gameName.c_str(), ARRAY_SIZE(req.arg.status.locationString));
679679
break;
680680
case BUDDY_LOADING:
681681
req.arg.status.status = GP_PLAYING;
682682
strcpy(req.arg.status.statusString, "Loading");
683-
sprintf(req.arg.status.locationString, "%s", gameName.c_str());
683+
strlcpy(req.arg.status.locationString, gameName.c_str(), ARRAY_SIZE(req.arg.status.locationString));
684684
break;
685685
case BUDDY_PLAYING:
686686
req.arg.status.status = GP_PLAYING;
687687
strcpy(req.arg.status.statusString, "Playing");
688-
sprintf(req.arg.status.locationString, "%s", gameName.c_str());
688+
strlcpy(req.arg.status.locationString, gameName.c_str(), ARRAY_SIZE(req.arg.status.locationString));
689689
break;
690690
case BUDDY_MATCHING:
691691
req.arg.status.status = GP_ONLINE;

‎Core/GameEngineDevice/Source/VideoDevice/Bink/BinkVideoPlayer.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ VideoStreamInterface* BinkVideoPlayer::open( AsciiString movieTitle )
229229
if (TheGlobalData->m_modDir.isNotEmpty())
230230
{
231231
char filePath[ _MAX_PATH ];
232-
sprintf( filePath, "%s%s\\%s.%s", TheGlobalData->m_modDir.str(), VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
232+
snprintf( filePath, ARRAY_SIZE(filePath), "%s%s\\%s.%s", TheGlobalData->m_modDir.str(), VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
233233
HBINK handle = BinkOpen(filePath , BINKPRELOADALL );
234234
DEBUG_ASSERTLOG(!handle, ("opened bink file %s", filePath));
235235
if (handle)
@@ -239,13 +239,13 @@ VideoStreamInterface* BinkVideoPlayer::open( AsciiString movieTitle )
239239
}
240240

241241
char localizedFilePath[ _MAX_PATH ];
242-
sprintf( localizedFilePath, VIDEO_LANG_PATH_FORMAT, GetRegistryLanguage().str(), pVideo->m_filename.str(), VIDEO_EXT );
242+
snprintf( localizedFilePath, ARRAY_SIZE(localizedFilePath), VIDEO_LANG_PATH_FORMAT, GetRegistryLanguage().str(), pVideo->m_filename.str(), VIDEO_EXT );
243243
HBINK handle = BinkOpen(localizedFilePath , BINKPRELOADALL );
244244
DEBUG_ASSERTLOG(!handle, ("opened localized bink file %s", localizedFilePath));
245245
if (!handle)
246246
{
247247
char filePath[ _MAX_PATH ];
248-
sprintf( filePath, "%s\\%s.%s", VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
248+
snprintf( filePath, ARRAY_SIZE(filePath), "%s\\%s.%s", VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
249249
handle = BinkOpen(filePath , BINKPRELOADALL );
250250
DEBUG_ASSERTLOG(!handle, ("opened bink file %s", localizedFilePath));
251251
}

‎Core/GameEngineDevice/Source/VideoDevice/FFmpeg/FFmpegVideoPlayer.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ VideoStreamInterface* FFmpegVideoPlayer::open( AsciiString movieTitle )
235235
if (TheGlobalData->m_modDir.isNotEmpty())
236236
{
237237
char filePath[ _MAX_PATH ];
238-
sprintf( filePath, "%s%s\\%s.%s", TheGlobalData->m_modDir.str(), VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
238+
snprintf( filePath, ARRAY_SIZE(filePath), "%s%s\\%s.%s", TheGlobalData->m_modDir.str(), VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
239239
File* file = TheFileSystem->openFile(filePath);
240240
DEBUG_ASSERTLOG(!file, ("opened bink file %s", filePath));
241241
if (file)
@@ -245,13 +245,13 @@ VideoStreamInterface* FFmpegVideoPlayer::open( AsciiString movieTitle )
245245
}
246246

247247
char localizedFilePath[ _MAX_PATH ];
248-
sprintf( localizedFilePath, VIDEO_LANG_PATH_FORMAT, GetRegistryLanguage().str(), pVideo->m_filename.str(), VIDEO_EXT );
248+
snprintf( localizedFilePath, ARRAY_SIZE(localizedFilePath), VIDEO_LANG_PATH_FORMAT, GetRegistryLanguage().str(), pVideo->m_filename.str(), VIDEO_EXT );
249249
File* file = TheFileSystem->openFile(localizedFilePath);
250250
DEBUG_ASSERTLOG(!file, ("opened localized bink file %s", localizedFilePath));
251251
if (!file)
252252
{
253253
char filePath[ _MAX_PATH ];
254-
sprintf( filePath, "%s\\%s.%s", VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
254+
snprintf( filePath, ARRAY_SIZE(filePath), "%s\\%s.%s", VIDEO_PATH, pVideo->m_filename.str(), VIDEO_EXT );
255255
file = TheFileSystem->openFile(filePath);
256256
DEBUG_ASSERTLOG(!file, ("opened bink file %s", filePath));
257257
}

‎Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DModelDraw.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -763,12 +763,12 @@ void ModelConditionInfo::validateWeaponBarrelInfo() const
763763

764764
if (!recoilBoneName.isEmpty())
765765
{
766-
sprintf(buffer, "%s%02d", recoilBoneName.str(), i);
766+
snprintf(buffer, ARRAY_SIZE(buffer), "%s%02d", recoilBoneName.str(), i);
767767
findPristineBone(NAMEKEY(buffer), &info.m_recoilBone);
768768
}
769769
if (!mfName.isEmpty())
770770
{
771-
sprintf(buffer, "%s%02d", mfName.str(), i);
771+
snprintf(buffer, ARRAY_SIZE(buffer), "%s%02d", mfName.str(), i);
772772
findPristineBone(NAMEKEY(buffer), &info.m_muzzleFlashBone);
773773
#if defined(RTS_DEBUG) || defined(DEBUG_CRASHING)
774774
if (info.m_muzzleFlashBone)
@@ -777,7 +777,7 @@ void ModelConditionInfo::validateWeaponBarrelInfo() const
777777
}
778778
if (!fxBoneName.isEmpty())
779779
{
780-
sprintf(buffer, "%s%02d", fxBoneName.str(), i);
780+
snprintf(buffer, ARRAY_SIZE(buffer), "%s%02d", fxBoneName.str(), i);
781781
findPristineBone(NAMEKEY(buffer), &info.m_fxBone);
782782
// special case: if we have multiple muzzleflashes, but only one fxbone, use that fxbone for everything.
783783
if (info.m_fxBone == 0 && info.m_muzzleFlashBone != 0)
@@ -787,7 +787,7 @@ void ModelConditionInfo::validateWeaponBarrelInfo() const
787787
Int plbBoneIndex = 0;
788788
if (!plbName.isEmpty())
789789
{
790-
sprintf(buffer, "%s%02d", plbName.str(), i);
790+
snprintf(buffer, ARRAY_SIZE(buffer), "%s%02d", plbName.str(), i);
791791
const Matrix3D* mtx = findPristineBone(NAMEKEY(buffer), &plbBoneIndex);
792792
if (mtx != nullptr)
793793
info.m_projectileOffsetMtx = *mtx;
@@ -3446,7 +3446,7 @@ Int W3DModelDraw::getPristineBonePositionsForConditionState(
34463446
if (i == 0)
34473447
strlcpy(buffer, boneNamePrefix, ARRAY_SIZE(buffer));
34483448
else
3449-
sprintf(buffer, "%s%02d", boneNamePrefix, i);
3449+
snprintf(buffer, ARRAY_SIZE(buffer), "%s%02d", boneNamePrefix, i);
34503450

34513451
for (char *c = buffer; c && *c; ++c)
34523452
{
@@ -3603,7 +3603,7 @@ Int W3DModelDraw::getCurrentBonePositions(
36033603
if (i == 0)
36043604
strlcpy(buffer, boneNamePrefix, ARRAY_SIZE(buffer));
36053605
else
3606-
sprintf(buffer, "%s%02d", boneNamePrefix, i);
3606+
snprintf(buffer, ARRAY_SIZE(buffer), "%s%02d", boneNamePrefix, i);
36073607

36083608
Int boneIndex = m_renderObject->Get_Bone_Index(buffer);
36093609
if (boneIndex == 0)

‎Core/GameEngineDevice/Source/W3DDevice/GameClient/Drawable/Draw/W3DSupplyDraw.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ void W3DSupplyDraw::updateDrawModuleSupplyStatus( Int maxSupply, Int currentSupp
9696
while( currentIndex <= highIndex )
9797
{
9898
char buffer[16];
99-
sprintf( buffer, "%s%02d", boneName.str(), currentIndex );
99+
snprintf( buffer, ARRAY_SIZE(buffer), "%s%02d", boneName.str(), currentIndex );
100100
ModelConditionInfo::HideShowSubObjInfo info;
101101
info.hide = hide;
102102
info.subObjName = buffer;

‎Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTreeBuffer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,10 +458,10 @@ void W3DTreeBuffer::updateTexture(void)
458458
for (i=0; i<m_numTreeTypes; i++) {
459459
char texturePath[ _MAX_PATH ];
460460
m_treeTypes[i].m_numTiles = 0;
461-
sprintf( texturePath, "%s%s", TERRAIN_TGA_DIR_PATH, m_treeTypes[i].m_data->m_textureName.str() );
461+
snprintf( texturePath, ARRAY_SIZE(texturePath), "%s%s", TERRAIN_TGA_DIR_PATH, m_treeTypes[i].m_data->m_textureName.str() );
462462
theFile = TheFileSystem->openFile( texturePath, File::READ|File::BINARY);
463463
if (theFile==nullptr) {
464-
sprintf( texturePath, "%s%s", TGA_DIR_PATH, m_treeTypes[i].m_data->m_textureName.str() );
464+
snprintf( texturePath, ARRAY_SIZE(texturePath), "%s%s", TGA_DIR_PATH, m_treeTypes[i].m_data->m_textureName.str() );
465465
theFile = TheFileSystem->openFile( texturePath, File::READ|File::BINARY);
466466
}
467467
if (theFile != nullptr) {

‎Core/GameEngineDevice/Source/W3DDevice/GameClient/WorldHeightMap.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1004,7 +1004,7 @@ void WorldHeightMap::readTexClass(TXTextureClass *texClass, TileData **tileData)
10041004
}
10051005
else
10061006
{
1007-
sprintf( texturePath, "%s%s", TERRAIN_TGA_DIR_PATH, terrain->getTexture().str() );
1007+
snprintf( texturePath, ARRAY_SIZE(texturePath), "%s%s", TERRAIN_TGA_DIR_PATH, terrain->getTexture().str() );
10081008
theFile = TheFileSystem->openFile( texturePath, File::READ|File::BINARY);
10091009
}
10101010

‎Core/Libraries/Source/WWVegas/WW3D2/FramGrab.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,15 +49,15 @@ FrameGrabClass::FrameGrabClass(const char *filename, MODE mode, int width, int h
4949
int result;
5050
char file[256];
5151
do {
52-
sprintf(file, "%s%d.AVI", filename, counter++);
52+
snprintf(file, ARRAY_SIZE(file), "%s%d.AVI", filename, counter++);
5353
result = _access(file, 0);
5454
} while(result != -1);
5555

5656
// Create new AVI file using AVIFileOpen.
5757
hr = AVIFileOpen(&AVIFile, file, OF_WRITE | OF_CREATE, nullptr);
5858
if (hr != 0) {
5959
char buf[256];
60-
sprintf(buf, "Unable to open %s\n", Filename);
60+
snprintf(buf, ARRAY_SIZE(buf), "Unable to open %s\n", Filename);
6161
OutputDebugString(buf);
6262
CleanupAVI();
6363
return;

0 commit comments

Comments
 (0)