Skip to content

fix(data): Sanitize GlobalData values after reading INI data #2519

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

Open
Cellcote wants to merge 1 commit into
base: main
Choose a base branch
from
+50 −8
Open

fix(data): Sanitize GlobalData values after reading INI data #2519

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 2 additions & 3 deletions Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DTerrainTracks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@
#include "shader.h"
#include "vertmaterial.h"
#include "Lib/BaseType.h"

#define MAX_TRACK_EDGE_COUNT 100 //maximum number of edges or divisions in track mark
#include "Common/GlobalData.h"
Copy link
Copy Markdown

@xezon xezon Apr 5, 2026

Choose a reason for hiding this comment

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

Tough to include GlobalData.h for just one define.

#define MAX_TRACK_OPAQUE_EDGE 25 //linear fade of edges will begin at this edge
#define FADE_TIME_FRAMES 300000 // 300 seconds at 30 fps - time to fade out an edge and remove it from the system.

Expand Down Expand Up @@ -86,7 +85,7 @@ class TerrainTracksRenderObjClass : public W3DMPO, public RenderObjClass
Real alpha; ///< current alpha value for rendering
};

edgeInfo m_edges[MAX_TRACK_EDGE_COUNT]; ///<edges at each segment break
edgeInfo m_edges[MAX_TANK_TRACK_EDGES]; ///<edges at each segment break
Vector3 m_lastAnchor; ///<location of last edge center
Int m_bottomIndex; ///<points at oldest edge on track
Int m_topIndex; ///<points to newest edge on track
Expand Down
2 changes: 1 addition & 1 deletion Core/GameEngineDevice/Source/W3DDevice/GameClient/W3DTerrainTracks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@
#include "WW3D2/dx8wrapper.h"
#include "WW3D2/scene.h"
#include "GameLogic/TerrainLogic.h"

Copy link
Copy Markdown

@xezon xezon Apr 5, 2026

Choose a reason for hiding this comment

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

Superfluous blank line

#include "GameLogic/Object.h"
#include "GameClient/Drawable.h"


#define BRIDGE_OFFSET_FACTOR 0.25f //amount to raise tracks above bridges.
//=============================================================================
// TerrainTracksRenderObjClass::~TerrainTracksRenderObjClass
Expand Down
2 changes: 2 additions & 0 deletions Generals/Code/GameEngine/Include/Common/GlobalData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ enum AIDebugOptions CPP_11(: Int);
// PUBLIC /////////////////////////////////////////////////////////////////////////////////////////

constexpr const Int MAX_GLOBAL_LIGHTS = 3;
constexpr const Int MAX_TANK_TRACK_EDGES = 100;
constexpr const Int ROAD_BUFFER_PADDING = 4;
constexpr const Int SIMULATE_REPLAYS_SEQUENTIAL = -1;

//-------------------------------------------------------------------------------------------------
Expand Down
20 changes: 20 additions & 0 deletions Generals/Code/GameEngine/Source/Common/GlobalData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1171,6 +1171,26 @@ void GlobalData::parseGameDataDefinition( INI* ini )
// parse the ini weapon definition
ini->initFromINI( TheWritableGlobalData, s_GlobalDataFieldParseTable );

// TheSuperHackers @fix Sanitize values that are used as loop bounds or allocation sizes.
TheWritableGlobalData->m_numGlobalLights = std::clamp(TheWritableGlobalData->m_numGlobalLights, 0, MAX_GLOBAL_LIGHTS);
TheWritableGlobalData->m_maxVisibleTranslucentObjects = std::max(TheWritableGlobalData->m_maxVisibleTranslucentObjects, 0);
TheWritableGlobalData->m_maxVisibleOccluderObjects = std::max(TheWritableGlobalData->m_maxVisibleOccluderObjects, 0);
TheWritableGlobalData->m_maxVisibleOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleOccludeeObjects, 0);
TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects, 0);
TheWritableGlobalData->m_maxLineBuildObjects = std::max(TheWritableGlobalData->m_maxLineBuildObjects, 0);
TheWritableGlobalData->m_maxRoadSegments = std::max(TheWritableGlobalData->m_maxRoadSegments, 0);
// Capped because DX8 vertex/index buffers use 16-bit indices and allocate size + ROAD_BUFFER_PADDING.
static constexpr Int MAX_ROAD_BUFFER_SIZE = 65535 - ROAD_BUFFER_PADDING;
static_assert(MAX_ROAD_BUFFER_SIZE > 0, "ROAD_BUFFER_PADDING must be less than 65535");
TheWritableGlobalData->m_maxRoadVertex = std::clamp(TheWritableGlobalData->m_maxRoadVertex, 0, MAX_ROAD_BUFFER_SIZE);
TheWritableGlobalData->m_maxRoadIndex = std::clamp(TheWritableGlobalData->m_maxRoadIndex, 0, MAX_ROAD_BUFFER_SIZE);
TheWritableGlobalData->m_maxRoadTypes = std::max(TheWritableGlobalData->m_maxRoadTypes, 0);
// Capped because TerrainTracksRenderObjClass has a fixed-size m_edges[MAX_TANK_TRACK_EDGES] array.
Copy link
Copy Markdown

@xezon xezon Apr 5, 2026

Choose a reason for hiding this comment

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

Better write m_edges[MAX_TANK_TRACK_EDGES] a different way, to not establish named code references in comments.

TheWritableGlobalData->m_maxTankTrackEdges = std::clamp(TheWritableGlobalData->m_maxTankTrackEdges, 1, MAX_TANK_TRACK_EDGES);
TheWritableGlobalData->m_networkFPSHistoryLength = std::max(TheWritableGlobalData->m_networkFPSHistoryLength, 1u);
TheWritableGlobalData->m_networkLatencyHistoryLength = std::max(TheWritableGlobalData->m_networkLatencyHistoryLength, 1u);
TheWritableGlobalData->m_networkCushionHistoryLength = std::max(TheWritableGlobalData->m_networkCushionHistoryLength, 1u);

TheWritableGlobalData->m_userDataDir.clear();
TheWritableGlobalData->m_userDataDir = BuildUserDataPathFromIni();
CreateDirectory(TheWritableGlobalData->m_userDataDir.str(), nullptr);
Expand Down
4 changes: 2 additions & 2 deletions Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DRoadBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,8 +179,8 @@ void RoadType::loadTexture(AsciiString path, Int ID)
m_roadTexture->Get_Filter().Set_U_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);
m_roadTexture->Get_Filter().Set_V_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);

m_vertexRoad=NEW_REF(DX8VertexBufferClass,(DX8_FVF_XYZDUV1,TheGlobalData->m_maxRoadVertex+4,DX8VertexBufferClass::USAGE_DYNAMIC));
m_indexRoad=NEW_REF(DX8IndexBufferClass,(TheGlobalData->m_maxRoadIndex+4, DX8IndexBufferClass::USAGE_DYNAMIC));
m_vertexRoad=NEW_REF(DX8VertexBufferClass,(DX8_FVF_XYZDUV1,TheGlobalData->m_maxRoadVertex+ROAD_BUFFER_PADDING,DX8VertexBufferClass::USAGE_DYNAMIC));
m_indexRoad=NEW_REF(DX8IndexBufferClass,(TheGlobalData->m_maxRoadIndex+ROAD_BUFFER_PADDING, DX8IndexBufferClass::USAGE_DYNAMIC));
m_numRoadVertices=0;
m_numRoadIndices=0;

Expand Down
2 changes: 2 additions & 0 deletions GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ enum AIDebugOptions CPP_11(: Int);
// PUBLIC /////////////////////////////////////////////////////////////////////////////////////////

constexpr const Int MAX_GLOBAL_LIGHTS = 3;
constexpr const Int MAX_TANK_TRACK_EDGES = 100;
constexpr const Int ROAD_BUFFER_PADDING = 4;
constexpr const Int SIMULATE_REPLAYS_SEQUENTIAL = -1;

//-------------------------------------------------------------------------------------------------
Expand Down
19 changes: 19 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1181,6 +1181,25 @@ void GlobalData::parseGameDataDefinition( INI* ini )
// parse the ini weapon definition
ini->initFromINI( TheWritableGlobalData, s_GlobalDataFieldParseTable );

// TheSuperHackers @fix Sanitize values that are used as loop bounds or allocation sizes.
TheWritableGlobalData->m_numGlobalLights = std::clamp(TheWritableGlobalData->m_numGlobalLights, 0, MAX_GLOBAL_LIGHTS);
TheWritableGlobalData->m_maxVisibleTranslucentObjects = std::max(TheWritableGlobalData->m_maxVisibleTranslucentObjects, 0);
TheWritableGlobalData->m_maxVisibleOccluderObjects = std::max(TheWritableGlobalData->m_maxVisibleOccluderObjects, 0);
TheWritableGlobalData->m_maxVisibleOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleOccludeeObjects, 0);
TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects, 0);
TheWritableGlobalData->m_maxLineBuildObjects = std::max(TheWritableGlobalData->m_maxLineBuildObjects, 0);
TheWritableGlobalData->m_maxRoadSegments = std::max(TheWritableGlobalData->m_maxRoadSegments, 0);
// Capped because DX8 vertex/index buffers use 16-bit indices and allocate size + ROAD_BUFFER_PADDING.
static constexpr Int MAX_ROAD_BUFFER_SIZE = 65535 - ROAD_BUFFER_PADDING;
Copy link
Copy Markdown

@xezon xezon Apr 5, 2026

Choose a reason for hiding this comment

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

constexpr const, because of VC6. No static.

static_assert(MAX_ROAD_BUFFER_SIZE > 0, "ROAD_BUFFER_PADDING must be less than 65535");
Copy link
Copy Markdown

@xezon xezon Apr 5, 2026

Choose a reason for hiding this comment

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

The assert condition does not match the message.

TheWritableGlobalData->m_maxRoadVertex = std::clamp(TheWritableGlobalData->m_maxRoadVertex, 0, MAX_ROAD_BUFFER_SIZE);
TheWritableGlobalData->m_maxRoadIndex = std::clamp(TheWritableGlobalData->m_maxRoadIndex, 0, MAX_ROAD_BUFFER_SIZE);
TheWritableGlobalData->m_maxRoadTypes = std::max(TheWritableGlobalData->m_maxRoadTypes, 0);
// Capped because TerrainTracksRenderObjClass has a fixed-size m_edges[MAX_TANK_TRACK_EDGES] array.
TheWritableGlobalData->m_maxTankTrackEdges = std::clamp(TheWritableGlobalData->m_maxTankTrackEdges, 1, MAX_TANK_TRACK_EDGES);
TheWritableGlobalData->m_networkFPSHistoryLength = std::max(TheWritableGlobalData->m_networkFPSHistoryLength, 1u);
TheWritableGlobalData->m_networkLatencyHistoryLength = std::max(TheWritableGlobalData->m_networkLatencyHistoryLength, 1u);
TheWritableGlobalData->m_networkCushionHistoryLength = std::max(TheWritableGlobalData->m_networkCushionHistoryLength, 1u);

// override INI values with user preferences
OptionPreferences optionPref;
Expand Down
4 changes: 2 additions & 2 deletions GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DRoadBuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,8 @@ void RoadType::loadTexture(AsciiString path, Int ID)
m_roadTexture->Get_Filter().Set_U_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);
m_roadTexture->Get_Filter().Set_V_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);

m_vertexRoad=NEW_REF(DX8VertexBufferClass,(DX8_FVF_XYZDUV1,TheGlobalData->m_maxRoadVertex+4, (s_dynamic?DX8VertexBufferClass::USAGE_DYNAMIC:DX8VertexBufferClass::USAGE_DEFAULT)));
m_indexRoad=NEW_REF(DX8IndexBufferClass,(TheGlobalData->m_maxRoadIndex+4, (s_dynamic?DX8IndexBufferClass::USAGE_DYNAMIC:DX8IndexBufferClass::USAGE_DEFAULT)));
m_vertexRoad=NEW_REF(DX8VertexBufferClass,(DX8_FVF_XYZDUV1,TheGlobalData->m_maxRoadVertex+ROAD_BUFFER_PADDING, (s_dynamic?DX8VertexBufferClass::USAGE_DYNAMIC:DX8VertexBufferClass::USAGE_DEFAULT)));
m_indexRoad=NEW_REF(DX8IndexBufferClass,(TheGlobalData->m_maxRoadIndex+ROAD_BUFFER_PADDING, (s_dynamic?DX8IndexBufferClass::USAGE_DYNAMIC:DX8IndexBufferClass::USAGE_DEFAULT)));
m_numRoadVertices=0;
m_numRoadIndices=0;

Expand Down