Skip to content

Commit a401e81

Browse files
CellcoteCellcote
committed
fix(data): Sanitize GlobalData values after reading INI data
Clamp m_numGlobalLights to [0, MAX_GLOBAL_LIGHTS] and floor allocation-size fields to prevent out-of-bounds array access and undefined behavior from bad INI values. Cap m_maxRoadVertex and m_maxRoadIndex to 16-bit DX8 buffer limit minus ROAD_BUFFER_PADDING, and m_maxTankTrackEdges to MAX_TANK_TRACK_EDGES. Move MAX_TRACK_EDGE_COUNT to GlobalData.h as MAX_TANK_TRACK_EDGES and replace magic +4 in W3DRoadBuffer with ROAD_BUFFER_PADDING constant.
1 parent 2d13736 commit a401e81

8 files changed

Lines changed: 50 additions & 8 deletions

File tree

‎Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DTerrainTracks.h‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,7 @@
3232
#include "shader.h"
3333
#include "vertmaterial.h"
3434
#include "Lib/BaseType.h"
35-
36-
#define MAX_TRACK_EDGE_COUNT 100 //maximum number of edges or divisions in track mark
35+
#include "Common/GlobalData.h"
3736
#define MAX_TRACK_OPAQUE_EDGE 25 //linear fade of edges will begin at this edge
3837
#define FADE_TIME_FRAMES 300000 // 300 seconds at 30 fps - time to fade out an edge and remove it from the system.
3938

@@ -86,7 +85,7 @@ class TerrainTracksRenderObjClass : public W3DMPO, public RenderObjClass
8685
Real alpha; ///< current alpha value for rendering
8786
};
8887

89-
edgeInfo m_edges[MAX_TRACK_EDGE_COUNT]; ///<edges at each segment break
88+
edgeInfo m_edges[MAX_TANK_TRACK_EDGES]; ///<edges at each segment break
9089
Vector3 m_lastAnchor; ///<location of last edge center
9190
Int m_bottomIndex; ///<points at oldest edge on track
9291
Int m_topIndex; ///<points to newest edge on track

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,10 @@
5757
#include "WW3D2/dx8wrapper.h"
5858
#include "WW3D2/scene.h"
5959
#include "GameLogic/TerrainLogic.h"
60+
6061
#include "GameLogic/Object.h"
6162
#include "GameClient/Drawable.h"
6263

63-
6464
#define BRIDGE_OFFSET_FACTOR 0.25f //amount to raise tracks above bridges.
6565
//=============================================================================
6666
// TerrainTracksRenderObjClass::~TerrainTracksRenderObjClass

‎Generals/Code/GameEngine/Include/Common/GlobalData.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ enum AIDebugOptions CPP_11(: Int);
5151
// PUBLIC /////////////////////////////////////////////////////////////////////////////////////////
5252

5353
constexpr const Int MAX_GLOBAL_LIGHTS = 3;
54+
constexpr const Int MAX_TANK_TRACK_EDGES = 100;
55+
constexpr const Int ROAD_BUFFER_PADDING = 4;
5456
constexpr const Int SIMULATE_REPLAYS_SEQUENTIAL = -1;
5557

5658
//-------------------------------------------------------------------------------------------------

‎Generals/Code/GameEngine/Source/Common/GlobalData.cpp‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1171,6 +1171,26 @@ void GlobalData::parseGameDataDefinition( INI* ini )
11711171
// parse the ini weapon definition
11721172
ini->initFromINI( TheWritableGlobalData, s_GlobalDataFieldParseTable );
11731173

1174+
// TheSuperHackers @fix Sanitize values that are used as loop bounds or allocation sizes.
1175+
TheWritableGlobalData->m_numGlobalLights = std::clamp(TheWritableGlobalData->m_numGlobalLights, 0, MAX_GLOBAL_LIGHTS);
1176+
TheWritableGlobalData->m_maxVisibleTranslucentObjects = std::max(TheWritableGlobalData->m_maxVisibleTranslucentObjects, 0);
1177+
TheWritableGlobalData->m_maxVisibleOccluderObjects = std::max(TheWritableGlobalData->m_maxVisibleOccluderObjects, 0);
1178+
TheWritableGlobalData->m_maxVisibleOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleOccludeeObjects, 0);
1179+
TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects, 0);
1180+
TheWritableGlobalData->m_maxLineBuildObjects = std::max(TheWritableGlobalData->m_maxLineBuildObjects, 0);
1181+
TheWritableGlobalData->m_maxRoadSegments = std::max(TheWritableGlobalData->m_maxRoadSegments, 0);
1182+
// Capped because DX8 vertex/index buffers use 16-bit indices and allocate size + ROAD_BUFFER_PADDING.
1183+
static constexpr Int MAX_ROAD_BUFFER_SIZE = 65535 - ROAD_BUFFER_PADDING;
1184+
static_assert(MAX_ROAD_BUFFER_SIZE > 0, "ROAD_BUFFER_PADDING must be less than 65535");
1185+
TheWritableGlobalData->m_maxRoadVertex = std::clamp(TheWritableGlobalData->m_maxRoadVertex, 0, MAX_ROAD_BUFFER_SIZE);
1186+
TheWritableGlobalData->m_maxRoadIndex = std::clamp(TheWritableGlobalData->m_maxRoadIndex, 0, MAX_ROAD_BUFFER_SIZE);
1187+
TheWritableGlobalData->m_maxRoadTypes = std::max(TheWritableGlobalData->m_maxRoadTypes, 0);
1188+
// Capped because TerrainTracksRenderObjClass has a fixed-size m_edges[MAX_TANK_TRACK_EDGES] array.
1189+
TheWritableGlobalData->m_maxTankTrackEdges = std::clamp(TheWritableGlobalData->m_maxTankTrackEdges, 1, MAX_TANK_TRACK_EDGES);
1190+
TheWritableGlobalData->m_networkFPSHistoryLength = std::max(TheWritableGlobalData->m_networkFPSHistoryLength, 1u);
1191+
TheWritableGlobalData->m_networkLatencyHistoryLength = std::max(TheWritableGlobalData->m_networkLatencyHistoryLength, 1u);
1192+
TheWritableGlobalData->m_networkCushionHistoryLength = std::max(TheWritableGlobalData->m_networkCushionHistoryLength, 1u);
1193+
11741194
TheWritableGlobalData->m_userDataDir.clear();
11751195
TheWritableGlobalData->m_userDataDir = BuildUserDataPathFromIni();
11761196
CreateDirectory(TheWritableGlobalData->m_userDataDir.str(), nullptr);

‎Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DRoadBuffer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,8 @@ void RoadType::loadTexture(AsciiString path, Int ID)
179179
m_roadTexture->Get_Filter().Set_U_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);
180180
m_roadTexture->Get_Filter().Set_V_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);
181181

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

‎GeneralsMD/Code/GameEngine/Include/Common/GlobalData.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ enum AIDebugOptions CPP_11(: Int);
5151
// PUBLIC /////////////////////////////////////////////////////////////////////////////////////////
5252

5353
constexpr const Int MAX_GLOBAL_LIGHTS = 3;
54+
constexpr const Int MAX_TANK_TRACK_EDGES = 100;
55+
constexpr const Int ROAD_BUFFER_PADDING = 4;
5456
constexpr const Int SIMULATE_REPLAYS_SEQUENTIAL = -1;
5557

5658
//-------------------------------------------------------------------------------------------------

‎GeneralsMD/Code/GameEngine/Source/Common/GlobalData.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1181,6 +1181,25 @@ void GlobalData::parseGameDataDefinition( INI* ini )
11811181
// parse the ini weapon definition
11821182
ini->initFromINI( TheWritableGlobalData, s_GlobalDataFieldParseTable );
11831183

1184+
// TheSuperHackers @fix Sanitize values that are used as loop bounds or allocation sizes.
1185+
TheWritableGlobalData->m_numGlobalLights = std::clamp(TheWritableGlobalData->m_numGlobalLights, 0, MAX_GLOBAL_LIGHTS);
1186+
TheWritableGlobalData->m_maxVisibleTranslucentObjects = std::max(TheWritableGlobalData->m_maxVisibleTranslucentObjects, 0);
1187+
TheWritableGlobalData->m_maxVisibleOccluderObjects = std::max(TheWritableGlobalData->m_maxVisibleOccluderObjects, 0);
1188+
TheWritableGlobalData->m_maxVisibleOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleOccludeeObjects, 0);
1189+
TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects = std::max(TheWritableGlobalData->m_maxVisibleNonOccluderOrOccludeeObjects, 0);
1190+
TheWritableGlobalData->m_maxLineBuildObjects = std::max(TheWritableGlobalData->m_maxLineBuildObjects, 0);
1191+
TheWritableGlobalData->m_maxRoadSegments = std::max(TheWritableGlobalData->m_maxRoadSegments, 0);
1192+
// Capped because DX8 vertex/index buffers use 16-bit indices and allocate size + ROAD_BUFFER_PADDING.
1193+
static constexpr Int MAX_ROAD_BUFFER_SIZE = 65535 - ROAD_BUFFER_PADDING;
1194+
static_assert(MAX_ROAD_BUFFER_SIZE > 0, "ROAD_BUFFER_PADDING must be less than 65535");
1195+
TheWritableGlobalData->m_maxRoadVertex = std::clamp(TheWritableGlobalData->m_maxRoadVertex, 0, MAX_ROAD_BUFFER_SIZE);
1196+
TheWritableGlobalData->m_maxRoadIndex = std::clamp(TheWritableGlobalData->m_maxRoadIndex, 0, MAX_ROAD_BUFFER_SIZE);
1197+
TheWritableGlobalData->m_maxRoadTypes = std::max(TheWritableGlobalData->m_maxRoadTypes, 0);
1198+
// Capped because TerrainTracksRenderObjClass has a fixed-size m_edges[MAX_TANK_TRACK_EDGES] array.
1199+
TheWritableGlobalData->m_maxTankTrackEdges = std::clamp(TheWritableGlobalData->m_maxTankTrackEdges, 1, MAX_TANK_TRACK_EDGES);
1200+
TheWritableGlobalData->m_networkFPSHistoryLength = std::max(TheWritableGlobalData->m_networkFPSHistoryLength, 1u);
1201+
TheWritableGlobalData->m_networkLatencyHistoryLength = std::max(TheWritableGlobalData->m_networkLatencyHistoryLength, 1u);
1202+
TheWritableGlobalData->m_networkCushionHistoryLength = std::max(TheWritableGlobalData->m_networkCushionHistoryLength, 1u);
11841203

11851204
// override INI values with user preferences
11861205
OptionPreferences optionPref;

‎GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DRoadBuffer.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ void RoadType::loadTexture(AsciiString path, Int ID)
183183
m_roadTexture->Get_Filter().Set_U_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);
184184
m_roadTexture->Get_Filter().Set_V_Addr_Mode(TextureFilterClass::TEXTURE_ADDRESS_REPEAT);
185185

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

0 commit comments

Comments
 (0)