Skip to content

fix: UB in movement path advancement #76

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
ZivDero merged 4 commits into from
Aug 30, 2026
+42 −31
Merged

fix: UB in movement path advancement #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1122338
Check path bounds before collision avoidance reads
Belonit Aug 30, 2026
661e204
Advance movement paths with overlap-safe copies
Belonit Aug 30, 2026
6a8c67d
Canonicalize generated movement paths
Belonit Aug 30, 2026
b6a391e
Handle tunnel markers in movement cost traversal
Belonit Aug 30, 2026
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
6 changes: 5 additions & 1 deletion code/astar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ double AStarClass::Get_Movement_Cost(CellClass **from, CellClass **to, bool brid
clear = true;
break;
}

if (facing == FACING_COUNT) {
break;
}
} else {
facing = occupier->PrimaryFacing.Current().As_Dir8();
}
Expand Down Expand Up @@ -900,7 +904,7 @@ void AStarClass::Apply_Path_Collision_Avoidance(FootClass * foot)
}

marked_path = true;
while (blocker->Path[index] != FACING_NONE && index < ARRAY_SIZE(blocker->Path)) {
while (index < ARRAY_SIZE(blocker->Path) && blocker->Path[index] != FACING_NONE) {
blocker_cell = Next_Cell(blocker_cell, blocker->Path[index]);
Map[blocker_cell].IsPredictedPath = !Map[blocker_cell].IsPredictedPath;
index++;
Expand Down
13 changes: 4 additions & 9 deletions code/drive.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1012,8 +1012,7 @@ bool DriveLocomotionClass::While_Moving(bool just_started)
TubeClass * tube = Tubes[tubenum];
Cell c = tube->Exit;
HeadToCoord = c.As_Coord();
memcpy((char*)&LinkedTo->Path, (char*)&LinkedTo->Path[1], sizeof(LinkedTo->Path[0]) * (CONQUER_PATH_MAX-1));
LinkedTo->Path[CONQUER_PATH_MAX-1] = FACING_NONE;
LinkedTo->Advance_Path(1);
LinkedTo->CurrentTube = tubenum;
LinkedTo->CurrentTubeDir = FACING_FIRST;
LinkedTo->LastTubeCoord = Map[Adjacent_Cell((Cell)tube->Enter, tube->Dirs[0])].Cell_Coord();
Expand Down Expand Up @@ -1196,8 +1195,7 @@ bool DriveLocomotionClass::While_Moving(bool just_started)
if (LinkedTo == NULL || !LinkedTo->IsActive || LinkedTo->IsInLimbo || LinkedTo->IsFalling) return(false);
if (Start_Driver(c)) {
LinkedTo->Set_Speed(oldspeed);
memcpy((char*)&LinkedTo->Path, (char*)&LinkedTo->Path[1], sizeof(LinkedTo->Path[0]) * (CONQUER_PATH_MAX-1));
LinkedTo->Path[CONQUER_PATH_MAX-1] = FACING_NONE;
LinkedTo->Advance_Path(1);
}
break;

Expand Down Expand Up @@ -1992,15 +1990,12 @@ bool DriveLocomotionClass::Start_Of_Move(bool & stop_processing, bool retry, boo
}

} else {
memcpy((char*)&LinkedTo->Path[0], (char*)&LinkedTo->Path[2], sizeof(LinkedTo->Path[0]) * (CONQUER_PATH_MAX-2));
LinkedTo->Path[CONQUER_PATH_MAX-2] = FACING_NONE;
LinkedTo->Advance_Path(2);
LinkedTo->IsPlanningToLook = true;
}
} else {
memcpy((char*)&LinkedTo->Path[0], (char*)&LinkedTo->Path[1], sizeof(LinkedTo->Path[0]) * (CONQUER_PATH_MAX-1));
LinkedTo->Advance_Path(1);
}

LinkedTo->Path[CONQUER_PATH_MAX-1] = FACING_NONE;
LinkedTo->LastPathingCell = dest.As_Cell();
LinkedTo->IsNewNavCom = false;
TrackIndex = 0;
Expand Down
34 changes: 26 additions & 8 deletions code/foot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,8 @@
#include "tube.hh"

#include <algorithm>
#include <cstring>
#include <iterator>


DynamicVectorClass<FootClass *> Feet;
Expand Down Expand Up @@ -195,7 +197,7 @@ FootClass::FootClass(HouseClass * house) :
CurrentTubeDir(0),
NextWaypoint(0)
{
Path[0] = FACING_NONE;
std::fill(std::begin(Path), std::end(Path), FACING_NONE);
Feet.Add(this);
TeamPtrTracker.Add(this);
}
Expand Down Expand Up @@ -427,12 +429,15 @@ bool FootClass::Basic_Path(Cell cell, int path_offset, int avoidance)
PathStruct * path; // Pointer to path control structure.
int skip_path = false;

if (path_offset == 0) {
Path[0] = FACING_NONE;
if (path_offset < 0 || path_offset >= ARRAY_SIZE(Path)) {
std::fill(std::begin(Path), std::end(Path), FACING_NONE);
return(false);
}

// Keep an existing path prefix, but always terminate the new suffix.
std::fill(std::begin(Path) + path_offset, std::end(Path), FACING_NONE);

if (!Is_In_Same_Zone(cell)) {
Path[0] = FACING_NONE;
return(false);
}

Expand Down Expand Up @@ -483,9 +488,6 @@ bool FootClass::Basic_Path(Cell cell, int path_offset, int avoidance)

if (!skip_path) {
Mark(MARK_UP);
if (path_offset == 0) {
Path[0] = FACING_NONE; // Probably not necessary, but...
}

/*
** Try to find a path to the destination. If a failure occurs, then keep trying
Expand All @@ -510,7 +512,8 @@ bool FootClass::Basic_Path(Cell cell, int path_offset, int avoidance)
*/
if (found) {
Fixup_Path(&path1);
memcpy(&Path[path_offset], &workpath[0], std::min(path->Length, ARRAY_SIZE(Path) - path_offset) * sizeof(Path[0]));
int length = std::clamp(path->Length, 0, ARRAY_SIZE(Path) - path_offset);
memcpy(&Path[path_offset], &workpath[0], length * sizeof(Path[0]));
}

Mark(MARK_DOWN);
Expand Down Expand Up @@ -581,6 +584,21 @@ bool FootClass::Basic_Path(Cell cell, int path_offset, int avoidance)
return(false);
}

/// <summary>
/// Removes completed steps from the front of the active movement path.
/// </summary>
void FootClass::Advance_Path(int count)
{
if (count <= 0) {
return;
}

int const advance = std::min(count, ARRAY_SIZE(Path));
std::memmove(Path, Path + advance, (ARRAY_SIZE(Path) - advance) * sizeof(Path[0]));
std::fill(std::end(Path) - advance, std::end(Path), FACING_NONE);
}



/***********************************************************************************************
* FootClass::Mission_Move -- AI process for moving a vehicle to its destination. *
Expand Down
1 change: 1 addition & 0 deletions code/foot.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ class FootClass : public TechnoClass
** Member function prototypes.
*/
bool Basic_Path(Cell cell, int path_offset = 0, int avoidance = 0);
void Advance_Path(int count);

virtual void Compute_CRC(CRCEngine &) const override;
virtual Coord Destination_Coord(void) const override;
Expand Down
6 changes: 2 additions & 4 deletions code/hover.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,9 +412,8 @@ MoveType HoverLocomotionClass::While_Moving(bool first_pass)
TubeClass * tube = Tubes[tubenum];
Cell exit = tube->Exit;
HeadToCoord = exit.As_Coord();
memcpy((char*)&LinkedTo->Path, (char*)&LinkedTo->Path[1], sizeof(LinkedTo->Path[0]) * (CONQUER_PATH_MAX-1));
LinkedTo->Advance_Path(1);
LinkedTo->LastPathingCell = HeadToCoord.As_Cell();
LinkedTo->Path[CONQUER_PATH_MAX-1] = FACING_NONE;
LinkedTo->CurrentTube = tubenum;
LinkedTo->CurrentTubeDir = FACING_FIRST;
LinkedTo->LastTubeCoord = Map[Adjacent_Cell((Cell)tube->Enter, tube->Dirs[0])].Cell_Coord() - Coord((Cell)tube->Enter) + LinkedTo->PositionCoord;
Expand Down Expand Up @@ -515,8 +514,7 @@ MoveType HoverLocomotionClass::While_Moving(bool first_pass)
if (LinkedTo->IsLocked) {
LinkedTo->Set_Occupy_Bit(HeadToCoord);
}
memcpy((char *)&LinkedTo->Path, (char *)&LinkedTo->Path[1], sizeof(LinkedTo->Path[0]) * (CONQUER_PATH_MAX - 1));
LinkedTo->Path[CONQUER_PATH_MAX - 1] = FACING_NONE;
LinkedTo->Advance_Path(1);
LinkedTo->LastPathingCell = HeadToCoord.As_Cell();
return(ok);

Expand Down
7 changes: 2 additions & 5 deletions code/mech.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ void MechLocomotionClass::Movement_AI(bool continue_moving)

if (HeadToCoord == COORD_NONE) {
if (DestinationCoord != COORD_NONE) {

/*
* A destination exists. If there is no current path then try to build
* one. If a path already exists, fall through to dispatch the next leg.
Expand Down Expand Up @@ -261,8 +260,7 @@ void MechLocomotionClass::Movement_AI(bool continue_moving)
Cell cell = tubeptr->Exit;
HeadToCoord = Coord(cell, 0);

memcpy(&LinkedTo->Path[0], &LinkedTo->Path[1], 92);
LinkedTo->Path[23] = FACING_NONE;
LinkedTo->Advance_Path(1);

LinkedTo->CurrentTube = tube;
LinkedTo->CurrentTubeDir = FACING_FIRST;
Expand Down Expand Up @@ -491,8 +489,7 @@ void MechLocomotionClass::Movement_AI(bool continue_moving)
LinkedTo->Path[1] = FACING_NONE;
}

memcpy(&LinkedTo->Path[0], &LinkedTo->Path[1], 92);
LinkedTo->Path[23] = FACING_NONE;
LinkedTo->Advance_Path(1);

LinkedTo->Set_Coord(HeadToCoord);
LinkedTo->LastPathingCell = HeadToCoord.As_Cell();
Expand Down
6 changes: 2 additions & 4 deletions code/walk.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,7 @@ void WalkLocomotionClass::Movement_AI(bool first_pass)
Cell cell = tube_ptr->Exit;
HeadToCoord = Coord(cell, 0);

memcpy(&LinkedTo->Path[0], &LinkedTo->Path[1], sizeof(LinkedTo->Path[0]) * (ARRAY_SIZE(LinkedTo->Path) - 1));
LinkedTo->Path[ARRAY_SIZE(LinkedTo->Path) - 1] = FACING_NONE;
LinkedTo->Advance_Path(1);

LinkedTo->CurrentTube = tube;
LinkedTo->CurrentTubeDir = FACING_FIRST;
Expand Down Expand Up @@ -459,8 +458,7 @@ void WalkLocomotionClass::Movement_AI(bool first_pass)
LinkedTo->Path[1] = FACING_NONE;
}

memcpy(&LinkedTo->Path[0], &LinkedTo->Path[1], sizeof(LinkedTo->Path[0]) * (ARRAY_SIZE(LinkedTo->Path) - 1));
LinkedTo->Path[ARRAY_SIZE(LinkedTo->Path) - 1] = FACING_NONE;
LinkedTo->Advance_Path(1);

LinkedTo->Set_Coord(HeadToCoord);
LinkedTo->LastPathingCell = HeadToCoord.As_Cell();
Expand Down
Loading