Skip to content

[GEN][ZH] Fix several potential buffer overruns in game code #849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 15, 2025
Merged

[GEN][ZH] Fix several potential buffer overruns in game code #849

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
e9985fd
Ensure FTP client has enough buffer space for local file name string.
slurmlord May 11, 2025
7e25d17
Enforce malloc was successful before dereferencing.
slurmlord May 11, 2025
8b9b2e8
Only copy colors in separate loop.
slurmlord May 12, 2025
44f7f6c
Give size in characters (not bytes) as buffer sizes to GetDateFormatW…
slurmlord May 12, 2025
bb92ee9
Assert the texture was created before dereferencing it.
slurmlord May 12, 2025
7d54e97
Ensure OverrideFlagLevel is within the array bounds.
slurmlord May 12, 2025
8887750
Undo previous unrelated once-here-fix.
slurmlord May 12, 2025
caa86e2
Revert previous assert, it was not needed.
slurmlord May 12, 2025
42988d6
Remove unnecessary assert.
slurmlord May 13, 2025
f968a81
Use ARRAY_SIZE macro for obtaining the element count of arrays.
slurmlord May 13, 2025
b021a4b
Don't write outside the surface buffer.
slurmlord May 13, 2025
d6aac19
Move clearing of m_currentD3DSurface to separate loop.
slurmlord May 13, 2025
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
3 changes: 0 additions & 3 deletions Core/Libraries/Source/WWVegas/WW3D2/dynamesh.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,9 +587,6 @@ DynamicMeshClass::DynamicMeshClass(const DynamicMeshClass & src) :

MultiVertexMaterial[pass] = src.MultiVertexMaterial[pass];
VertexMaterialIdx[pass] = src.VertexMaterialIdx[pass];

MultiVertexColor[pass] = src.MultiVertexColor[pass];
CurVertexColor[pass] = src.CurVertexColor[pass];
}

for (int color_array_index = 0; color_array_index < MAX_COLOR_ARRAYS; color_array_index++) {
Expand Down
2 changes: 1 addition & 1 deletion Core/Libraries/Source/WWVegas/WWDownload/ftp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class Cftp
char m_szRemoteFilePath[128];
char m_szRemoteFileName[128];
char m_szLocalFilePath[128];
char m_szLocalFileName[128];
char m_szLocalFileName[256];
char m_szServerName[128];
char m_szUserName[128];
char m_szPassword[128];
Expand Down
4 changes: 2 additions & 2 deletions Generals/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ UnicodeString getUnicodeDateBuffer(SYSTEMTIME timeVal)
DATE_SHORTDATE,
&timeVal,
NULL,
dateBuffer, sizeof(dateBuffer) );
dateBuffer, ARRAY_SIZE(dateBuffer) );
displayDateBuffer.set(dateBuffer);
return displayDateBuffer;
//displayDateBuffer.format( L"%ls", dateBuffer );
Expand Down Expand Up @@ -270,7 +270,7 @@ UnicodeString getUnicodeTimeBuffer(SYSTEMTIME timeVal)
&timeVal,
NULL,
timeBuffer,
sizeof(timeBuffer) );
ARRAY_SIZE(timeBuffer) );
displayTimeBuffer.set(timeBuffer);
return displayTimeBuffer;
}
Expand Down
4 changes: 2 additions & 2 deletions Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupReplay.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ void reallySaveReplay(void)
if(DeleteFile(filename.str()) == 0)
{
wchar_t buffer[1024];
FormatMessageW ( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, sizeof(buffer), NULL);
FormatMessageW ( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, ARRAY_SIZE(buffer), NULL);
UnicodeString errorStr;
errorStr.set(buffer);
errorStr.trim();
Expand All @@ -313,7 +313,7 @@ void reallySaveReplay(void)
if(CopyFile(oldFilename.str(),filename.str(), FALSE) == 0)
{
wchar_t buffer[1024];
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, sizeof(buffer), NULL);
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, ARRAY_SIZE(buffer), NULL);
UnicodeString errorStr;
errorStr.set(buffer);
errorStr.trim();
Expand Down
2 changes: 1 addition & 1 deletion Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ void copyReplay( void )
if(CopyFile(filename.str(),newFilename.str(), FALSE) == 0)
{
wchar_t buffer[1024];
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, sizeof(buffer), NULL);
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, ARRAY_SIZE(buffer), NULL);
UnicodeString errorStr;
errorStr.set(buffer);
errorStr.trim();
Expand Down
2 changes: 1 addition & 1 deletion Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DMouse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ void W3DMouse::initD3DAssets(void)
for (Int j=0; j < MAX_2D_CURSOR_ANIM_FRAMES; j++)
{
cursorTextures[i][j]=NULL;//am->Get_Texture(m_cursorInfo[i].textureName.str());
m_currentD3DSurface[i]=NULL;
m_currentD3DSurface[j]=NULL;
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion Generals/Code/Libraries/Source/WWVegas/WW3D2/rinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ MaterialPassClass * RenderInfoClass::Peek_Additional_Pass(int i)
void RenderInfoClass::Push_Override_Flags(RINFO_OVERRIDE_FLAGS flg)
{
// copy to the end of the array
WWASSERT(OverrideFlagLevel<MAX_OVERRIDE_FLAG_LEVEL);
WWASSERT(OverrideFlagLevel<MAX_OVERRIDE_FLAG_LEVEL - 1);
OverrideFlagLevel++;
OverrideFlag[OverrideFlagLevel]=flg;
}
Expand Down
4 changes: 2 additions & 2 deletions GeneralsMD/Code/GameEngine/Source/Common/System/SaveGame/GameState.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ UnicodeString getUnicodeDateBuffer(SYSTEMTIME timeVal)
DATE_SHORTDATE,
&timeVal,
NULL,
dateBuffer, sizeof(dateBuffer) );
dateBuffer, ARRAY_SIZE(dateBuffer) );
displayDateBuffer.set(dateBuffer);
return displayDateBuffer;
//displayDateBuffer.format( L"%ls", dateBuffer );
Expand Down Expand Up @@ -270,7 +270,7 @@ UnicodeString getUnicodeTimeBuffer(SYSTEMTIME timeVal)
&timeVal,
NULL,
timeBuffer,
sizeof(timeBuffer) );
ARRAY_SIZE(timeBuffer) );
displayTimeBuffer.set(timeBuffer);
return displayTimeBuffer;
}
Expand Down
4 changes: 2 additions & 2 deletions GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/PopupReplay.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ void reallySaveReplay(void)
if(DeleteFile(filename.str()) == 0)
{
wchar_t buffer[1024];
FormatMessageW ( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, sizeof(buffer), NULL);
FormatMessageW ( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, ARRAY_SIZE(buffer), NULL);
UnicodeString errorStr;
errorStr.set(buffer);
errorStr.trim();
Expand All @@ -313,7 +313,7 @@ void reallySaveReplay(void)
if(CopyFile(oldFilename.str(),filename.str(), FALSE) == 0)
{
wchar_t buffer[1024];
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, sizeof(buffer), NULL);
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, ARRAY_SIZE(buffer), NULL);
UnicodeString errorStr;
errorStr.set(buffer);
errorStr.trim();
Expand Down
2 changes: 1 addition & 1 deletion GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/ReplayMenu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ void copyReplay( void )
if(CopyFile(filename.str(),newFilename.str(), FALSE) == 0)
{
wchar_t buffer[1024];
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, sizeof(buffer), NULL);
FormatMessageW( FORMAT_MESSAGE_FROM_SYSTEM, NULL, GetLastError(), 0, buffer, ARRAY_SIZE(buffer), NULL);
UnicodeString errorStr;
errorStr.set(buffer);
errorStr.trim();
Expand Down
2 changes: 1 addition & 1 deletion GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DMouse.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ void W3DMouse::initD3DAssets(void)
for (Int j=0; j < MAX_2D_CURSOR_ANIM_FRAMES; j++)
{
cursorTextures[i][j]=NULL;//am->Get_Texture(m_cursorInfo[i].textureName.str());
m_currentD3DSurface[i]=NULL;
m_currentD3DSurface[j]=NULL;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double loop is not the correct location to write to this member.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, it was a compromise in order to keep the diff as minimal as possible. Went with a separate loop for clearing m_current_D3DSurface instead.

}
}
}
Expand Down
2 changes: 1 addition & 1 deletion GeneralsMD/Code/Libraries/Source/WWVegas/WW3D2/rinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ MaterialPassClass * RenderInfoClass::Peek_Additional_Pass(int i)
void RenderInfoClass::Push_Override_Flags(RINFO_OVERRIDE_FLAGS flg)
{
// copy to the end of the array
WWASSERT(OverrideFlagLevel<MAX_OVERRIDE_FLAG_LEVEL);
WWASSERT(OverrideFlagLevel<MAX_OVERRIDE_FLAG_LEVEL - 1);
OverrideFlagLevel++;
OverrideFlag[OverrideFlagLevel]=flg;
}
Expand Down
Loading