Skip to content

Commit 5499057

Browse files
PartitionManager: Add safety checks to prevent access violations in iterateCellsBreadthFirst and getNearestGroupWithValue
1 parent bae4229 commit 5499057

2 files changed

Lines changed: 134 additions & 32 deletions

File tree

Generals/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp

Lines changed: 67 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4402,12 +4402,41 @@ Int PartitionManager::iterateCellsBreadthFirst(const Coord3D *pos, CellBreadthFi
44024402
// Call proc on each cell, and if it returns non-zero, then return the cell index
44034403
// -1 means error, but we should add a define later for this.
44044404

4405+
// Validate input parameters to prevent access violations
4406+
if (!pos) {
4407+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: pos parameter is null"));
4408+
return -1;
4409+
}
4410+
4411+
if (!m_cells) {
4412+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: m_cells array is null"));
4413+
return -1;
4414+
}
4415+
4416+
if (!proc) {
4417+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: proc parameter is null"));
4418+
return -1;
4419+
}
4420+
44054421
Int cellX, cellY;
44064422
ThePartitionManager->worldToCell(pos->x, pos->y, &cellX, &cellY);
44074423

4424+
// Validate that the starting cell coordinates are within bounds
4425+
if (cellX < 0 || cellX >= m_cellCountX || cellY < 0 || cellY >= m_cellCountY) {
4426+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: starting cell (%d, %d) is out of bounds (max: %d, %d)", cellX, cellY, m_cellCountX, m_cellCountY));
4427+
return -1;
4428+
}
4429+
44084430
// Note, bool. not Bool, cause bool will cause this to be a bitfield.
44094431
std::vector<bool> bitField;
44104432
Int cellCount = m_cellCountX * m_cellCountY;
4433+
4434+
// Validate cellCount is valid
4435+
if (cellCount <= 0) {
4436+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: invalid cell count %d", cellCount));
4437+
return -1;
4438+
}
4439+
44114440
bitField.resize(cellCount);
44124441
// 0 means not done, 1 means done.
44134442

@@ -4418,48 +4447,64 @@ Int PartitionManager::iterateCellsBreadthFirst(const Coord3D *pos, CellBreadthFi
44184447
std::queue<PartitionCell *> cellQ;
44194448

44204449
// mark the starting cell done
4421-
bitField[cellY * m_cellCountX + cellX] = true;
4422-
cellQ.push(&m_cells[cellY * m_cellCountX + cellX]);
4450+
Int startIndex = cellY * m_cellCountX + cellX;
4451+
if (startIndex >= 0 && startIndex < cellCount) {
4452+
bitField[startIndex] = true;
4453+
cellQ.push(&m_cells[startIndex]);
4454+
} else {
4455+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: starting index %d is out of bounds", startIndex));
4456+
return -1;
4457+
}
44234458

44244459
Int curX = cellX;
44254460
Int curY = cellY;
44264461
while (!cellQ.empty()) {
44274462
// first, add the neighbors
44284463
// left
44294464
if (curX - 1 >= 0) {
4430-
if (!bitField[curY * m_cellCountX + curX - 1]) {
4431-
bitField[curY * m_cellCountX + curX - 1] = true;
4432-
cellQ.push(&m_cells[curY * m_cellCountX + curX - 1]);
4465+
Int leftIndex = curY * m_cellCountX + curX - 1;
4466+
if (leftIndex >= 0 && leftIndex < cellCount && !bitField[leftIndex]) {
4467+
bitField[leftIndex] = true;
4468+
cellQ.push(&m_cells[leftIndex]);
44334469
}
44344470
}
44354471

44364472
// top
44374473
if (curY - 1 >= 0) {
4438-
if (!bitField[(curY - 1) * m_cellCountX + curX]) {
4439-
bitField[(curY - 1) * m_cellCountX + curX] = true;
4440-
cellQ.push(&m_cells[(curY - 1) * m_cellCountX + curX]);
4474+
Int topIndex = (curY - 1) * m_cellCountX + curX;
4475+
if (topIndex >= 0 && topIndex < cellCount && !bitField[topIndex]) {
4476+
bitField[topIndex] = true;
4477+
cellQ.push(&m_cells[topIndex]);
44414478
}
44424479
}
44434480

44444481
// right
44454482
if (curX + 1 < m_cellCountX) {
4446-
if (!bitField[curY * m_cellCountX + curX + 1]) {
4447-
bitField[curY * m_cellCountX + curX + 1] = true;
4448-
cellQ.push(&m_cells[curY * m_cellCountX + curX + 1]);
4483+
Int rightIndex = curY * m_cellCountX + curX + 1;
4484+
if (rightIndex >= 0 && rightIndex < cellCount && !bitField[rightIndex]) {
4485+
bitField[rightIndex] = true;
4486+
cellQ.push(&m_cells[rightIndex]);
44494487
}
44504488
}
44514489

44524490
// bottom
4453-
if (curY + 1 > 0) {
4454-
if (!bitField[(curY + 1) * m_cellCountX + curX]) {
4455-
bitField[(curY + 1) * m_cellCountX + curX] = true;
4456-
cellQ.push(&m_cells[(curY + 1) * m_cellCountX + curX]);
4491+
if (curY + 1 < m_cellCountY) {
4492+
Int bottomIndex = (curY + 1) * m_cellCountX + curX;
4493+
if (bottomIndex >= 0 && bottomIndex < cellCount && !bitField[bottomIndex]) {
4494+
bitField[bottomIndex] = true;
4495+
cellQ.push(&m_cells[bottomIndex]);
44574496
}
44584497
}
44594498

44604499
// now process the current top.
44614500
PartitionCell *curCell = cellQ.front();
44624501
cellQ.pop();
4502+
4503+
if (!curCell) {
4504+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: curCell is null"));
4505+
continue;
4506+
}
4507+
44634508
curX = curCell->getCellX();
44644509
curY = curCell->getCellY();
44654510

@@ -4781,6 +4826,12 @@ void PartitionManager::getNearestGroupWithValue( Int playerIndex, UnsignedInt wh
47814826
if (!(sourceLocation && outLocation))
47824827
return;
47834828

4829+
// Additional validation to prevent crashes
4830+
if (!m_cells) {
4831+
DEBUG_CRASH(("PartitionManager::getNearestGroupWithValue: m_cells array is null"));
4832+
return;
4833+
}
4834+
47844835
PlayerMaskType playerMask = ThePlayerList->getPlayersWithRelationship(playerIndex, whichPlayerTypes);
47854836
if (playerMask == 0)
47864837
return;
@@ -4807,7 +4858,7 @@ void PartitionManager::getNearestGroupWithValue( Int playerIndex, UnsignedInt wh
48074858
parms.allPlayersMask[i] = allPlayerMasks[i];
48084859

48094860
Int nearestGreat = iterateCellsBreadthFirst(sourceLocation, cellValueProc, &parms);
4810-
if (nearestGreat != -1) {
4861+
if (nearestGreat != -1 && nearestGreat >= 0 && nearestGreat < m_totalCellCount) {
48114862
(*outLocation).x = m_cells[nearestGreat].getCellX() * TheGlobalData->m_partitionCellSize;
48124863
(*outLocation).y = m_cells[nearestGreat].getCellY() * TheGlobalData->m_partitionCellSize;
48134864
(*outLocation).z = 0;

GeneralsMD/Code/GameEngine/Source/GameLogic/Object/PartitionManager.cpp

Lines changed: 67 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4438,12 +4438,41 @@ Int PartitionManager::iterateCellsBreadthFirst(const Coord3D *pos, CellBreadthFi
44384438
// Call proc on each cell, and if it returns non-zero, then return the cell index
44394439
// -1 means error, but we should add a define later for this.
44404440

4441+
// Validate input parameters to prevent access violations
4442+
if (!pos) {
4443+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: pos parameter is null"));
4444+
return -1;
4445+
}
4446+
4447+
if (!m_cells) {
4448+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: m_cells array is null"));
4449+
return -1;
4450+
}
4451+
4452+
if (!proc) {
4453+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: proc parameter is null"));
4454+
return -1;
4455+
}
4456+
44414457
Int cellX, cellY;
44424458
ThePartitionManager->worldToCell(pos->x, pos->y, &cellX, &cellY);
44434459

4460+
// Validate that the starting cell coordinates are within bounds
4461+
if (cellX < 0 || cellX >= m_cellCountX || cellY < 0 || cellY >= m_cellCountY) {
4462+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: starting cell (%d, %d) is out of bounds (max: %d, %d)", cellX, cellY, m_cellCountX, m_cellCountY));
4463+
return -1;
4464+
}
4465+
44444466
// Note, bool. not Bool, cause bool will cause this to be a bitfield.
44454467
std::vector<bool> bitField;
44464468
Int cellCount = m_cellCountX * m_cellCountY;
4469+
4470+
// Validate cellCount is valid
4471+
if (cellCount <= 0) {
4472+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: invalid cell count %d", cellCount));
4473+
return -1;
4474+
}
4475+
44474476
bitField.resize(cellCount);
44484477
// 0 means not done, 1 means done.
44494478

@@ -4454,48 +4483,64 @@ Int PartitionManager::iterateCellsBreadthFirst(const Coord3D *pos, CellBreadthFi
44544483
std::queue<PartitionCell *> cellQ;
44554484

44564485
// mark the starting cell done
4457-
bitField[cellY * m_cellCountX + cellX] = true;
4458-
cellQ.push(&m_cells[cellY * m_cellCountX + cellX]);
4486+
Int startIndex = cellY * m_cellCountX + cellX;
4487+
if (startIndex >= 0 && startIndex < cellCount) {
4488+
bitField[startIndex] = true;
4489+
cellQ.push(&m_cells[startIndex]);
4490+
} else {
4491+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: starting index %d is out of bounds", startIndex));
4492+
return -1;
4493+
}
44594494

44604495
Int curX = cellX;
44614496
Int curY = cellY;
44624497
while (!cellQ.empty()) {
44634498
// first, add the neighbors
44644499
// left
44654500
if (curX - 1 >= 0) {
4466-
if (!bitField[curY * m_cellCountX + curX - 1]) {
4467-
bitField[curY * m_cellCountX + curX - 1] = true;
4468-
cellQ.push(&m_cells[curY * m_cellCountX + curX - 1]);
4501+
Int leftIndex = curY * m_cellCountX + curX - 1;
4502+
if (leftIndex >= 0 && leftIndex < cellCount && !bitField[leftIndex]) {
4503+
bitField[leftIndex] = true;
4504+
cellQ.push(&m_cells[leftIndex]);
44694505
}
44704506
}
44714507

44724508
// top
44734509
if (curY - 1 >= 0) {
4474-
if (!bitField[(curY - 1) * m_cellCountX + curX]) {
4475-
bitField[(curY - 1) * m_cellCountX + curX] = true;
4476-
cellQ.push(&m_cells[(curY - 1) * m_cellCountX + curX]);
4510+
Int topIndex = (curY - 1) * m_cellCountX + curX;
4511+
if (topIndex >= 0 && topIndex < cellCount && !bitField[topIndex]) {
4512+
bitField[topIndex] = true;
4513+
cellQ.push(&m_cells[topIndex]);
44774514
}
44784515
}
44794516

44804517
// right
44814518
if (curX + 1 < m_cellCountX) {
4482-
if (!bitField[curY * m_cellCountX + curX + 1]) {
4483-
bitField[curY * m_cellCountX + curX + 1] = true;
4484-
cellQ.push(&m_cells[curY * m_cellCountX + curX + 1]);
4519+
Int rightIndex = curY * m_cellCountX + curX + 1;
4520+
if (rightIndex >= 0 && rightIndex < cellCount && !bitField[rightIndex]) {
4521+
bitField[rightIndex] = true;
4522+
cellQ.push(&m_cells[rightIndex]);
44854523
}
44864524
}
44874525

44884526
// bottom
4489-
if (curY + 1 > 0) {
4490-
if (!bitField[(curY + 1) * m_cellCountX + curX]) {
4491-
bitField[(curY + 1) * m_cellCountX + curX] = true;
4492-
cellQ.push(&m_cells[(curY + 1) * m_cellCountX + curX]);
4527+
if (curY + 1 < m_cellCountY) {
4528+
Int bottomIndex = (curY + 1) * m_cellCountX + curX;
4529+
if (bottomIndex >= 0 && bottomIndex < cellCount && !bitField[bottomIndex]) {
4530+
bitField[bottomIndex] = true;
4531+
cellQ.push(&m_cells[bottomIndex]);
44934532
}
44944533
}
44954534

44964535
// now process the current top.
44974536
PartitionCell *curCell = cellQ.front();
44984537
cellQ.pop();
4538+
4539+
if (!curCell) {
4540+
DEBUG_CRASH(("PartitionManager::iterateCellsBreadthFirst: curCell is null"));
4541+
continue;
4542+
}
4543+
44994544
curX = curCell->getCellX();
45004545
curY = curCell->getCellY();
45014546

@@ -4817,6 +4862,12 @@ void PartitionManager::getNearestGroupWithValue( Int playerIndex, UnsignedInt wh
48174862
if (!(sourceLocation && outLocation))
48184863
return;
48194864

4865+
// Additional validation to prevent crashes
4866+
if (!m_cells) {
4867+
DEBUG_CRASH(("PartitionManager::getNearestGroupWithValue: m_cells array is null"));
4868+
return;
4869+
}
4870+
48204871
PlayerMaskType playerMask = ThePlayerList->getPlayersWithRelationship(playerIndex, whichPlayerTypes);
48214872
if (playerMask == 0)
48224873
return;
@@ -4843,7 +4894,7 @@ void PartitionManager::getNearestGroupWithValue( Int playerIndex, UnsignedInt wh
48434894
parms.allPlayersMask[i] = allPlayerMasks[i];
48444895

48454896
Int nearestGreat = iterateCellsBreadthFirst(sourceLocation, cellValueProc, &parms);
4846-
if (nearestGreat != -1) {
4897+
if (nearestGreat != -1 && nearestGreat >= 0 && nearestGreat < m_totalCellCount) {
48474898
(*outLocation).x = m_cells[nearestGreat].getCellX() * TheGlobalData->m_partitionCellSize;
48484899
(*outLocation).y = m_cells[nearestGreat].getCellY() * TheGlobalData->m_partitionCellSize;
48494900
(*outLocation).z = 0;

0 commit comments

Comments
 (0)