[SP] Crash when filling SteerUser neighbors (out of bounds vector access)

[V-Sarthou]

We loop over EntityList, which can contain MAX_GENTITIES values, but mNeighbors can only contain STEER::MAX_NEIGHBORS values.

I encountered the crash on Windows x64 (with a custom build based on 90e8005b0cef3ce3405ad3f0011a204ca78e4c37) by using the following commands:

helpusobi 1
npc spawn kyle
npc spawn reborn (~33 times)

Here is the complete callstack i got in Debug:

jagamex86_64.dll!ratl::vector_base<ratl::storage::value_semantics<gentity_s *,20>>::push_back(gentity_s * const & value) code/Ratl/vector_vs.h line 205
jagamex86_64.dll!STEER::Activate(gentity_s * actor) code/game/g_navigator.cpp line 4165
jagamex86_64.dll!NPC_MoveToGoal(qboolean tryStraight) code/game/NPC_move.cpp line 805
jagamex86_64.dll!Jedi_Hunt() code/game/AI_Jedi.cpp line 1061
jagamex86_64.dll!Jedi_Combat() code/game/AI_Jedi.cpp line 5872
jagamex86_64.dll!Jedi_Attack() code/game/AI_Jedi.cpp line 6871
jagamex86_64.dll!NPC_BSJedi_Default() code/game/AI_Jedi.cpp line 7645
jagamex86_64.dll!NPC_BehaviorSet_Jedi(int bState) code/game/NPC.cpp line 1594
jagamex86_64.dll!NPC_RunBehavior(int team, int bState) code/game/NPC.cpp line 1898
jagamex86_64.dll!NPC_ExecuteBState(gentity_s * self) code/game/NPC.cpp line 2203
jagamex86_64.dll!NPC_Think(gentity_s * self) code/game/NPC.cpp line 2501
jagamex86_64.dll!GEntity_ThinkFunc(gentity_s * self) code/game/g_functions.cpp line 67
jagamex86_64.dll!G_RunThink(gentity_s * ent) code/game/g_main.cpp line 1078
jagamex86_64.dll!G_RunFrame(int levelTime) code/game/g_main.cpp line 2056
openjk_sp.x86_64.exe!SV_Frame(int msec, float fractionMsec) code/server/sv_main.cpp line 514
openjk_sp.x86_64.exe!Com_Frame() code/qcommon/common.cpp line 1432
openjk_sp.x86_64.exe!SDL_main(int argc, char * * argv) shared/sys/sys_main.cpp line 813
openjk_sp.x86_64.exe!main_getcmdline() line 71

The assert in the push_back() function fails because mSize == CAPACITY (==20)

I am not sure if this is the most appropriate way to fix the crash, but I still wanted to share this find.

[ensiform]

I wonder if it makes more sense to prevent the push_back if full() already. Though I do not know if that in and of itself is the most correct fix still.

[V-Sarthou]

Hello and thank you for your response,

The mNeighbors vector is cleared just before the loop, and the only side effect of the loop is the push_back() in the vector, so it made sense to me to just exit the loop as soon as the vector is full, since continuing the loop would be useless.

I guess the selected neighbors are not guaranteed to be the "best" neighbors out of the bigger list returned by gi.EntitiesInBox(). At least, it does not change the current behavior in non-crashing scenarios.

[xiaoxiao921]

Sorting the EntitiesInBox() output before iterating it seems to be a change in behaviour.

Another way of fixing this is to edit the for loop condition to i<numFound && !suser.mNeighbors.full() but thats just preference at this point.