Skip to content

Commit

Permalink
Duke: add range checks for accessing the moves array
Browse files Browse the repository at this point in the history
CON can easily produce bad indices due to sloppy design.
In ZScript always throw an exception if such an index is out of bounds.
  • Loading branch information
coelckers committed Oct 9, 2024
1 parent 48d4545 commit 1dedb43
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 4 deletions.
4 changes: 2 additions & 2 deletions source/games/duke/src/gameexec.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1528,7 +1528,7 @@ int ParseState::parse(void)
auto ai = &ais[*insptr];
g_ac->curAI = ai->name;
g_ac->curAction = &actions[ai->action];
g_ac->curMove = &moves[ai->move];
g_ac->curMove = &moves[ai->move >= 0 && ai->move < moves.Size()? ai->move : 0];
g_ac->spr.hitag = ai->moveflags;
g_ac->actioncounter = g_ac->curframe = 0;
g_ac->counter = 0;
Expand Down Expand Up @@ -1874,7 +1874,7 @@ int ParseState::parse(void)
case concmd_move:
g_ac->counter=0;
insptr++;
g_ac->curMove = &moves[*insptr];
g_ac->curMove = &moves[*insptr >= 0 && *insptr < moves.Size() ? *insptr : 0];
insptr++;
g_ac->spr.hitag = *insptr;
insptr++;
Expand Down
4 changes: 2 additions & 2 deletions source/games/duke/src/spawn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,15 @@ static void initanimations(DDukeActor* act)
act->actioncounter = act->curframe = 0;

ndx = LookupMove(act->GetClass(), ainf->DefaultMove);
act->curMove = &moves[ndx];
act->curMove = &moves[ndx >= 0 && ndx < moves.Size() ? ndx : 0];
if (ainf->DefaultMoveflags && act->spr.hitag == 0)
act->spr.hitag = ainf->DefaultMoveflags;
}
else if (coninf)
{
auto sa = &ScriptCode[coninf->scriptaddress];
act->curAction = &actions[sa[1]];
act->curMove = &moves[sa[2]];
act->curMove = &moves[sa[2] >= 0 && sa[2] < moves.Size() ? sa[2] : 0];
int s3 = sa[3];
if (s3 && act->spr.hitag == 0)
act->spr.hitag = s3;
Expand Down
5 changes: 5 additions & 0 deletions source/games/duke/src/vmexports.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ DEFINE_ACTION_FUNCTION_NATIVE(DDukeActor, aim, aim_)
void Duke_SetAction(DDukeActor* self, int intname)
{
int ndx = LookupAction(self->GetClass(), FName(ENamedName(intname)));
if (ndx < 0 || ndx >= actions.SSize()) ThrowAbortException(X_ARRAY_OUT_OF_BOUNDS, "Bad 'action' index");
self->curAction = &actions[ndx];
self->actioncounter = self->curframe = 0;

Expand All @@ -749,6 +750,7 @@ DEFINE_ACTION_FUNCTION_NATIVE(DDukeActor, SetAction, Duke_SetAction)
void Duke_SetMove(DDukeActor* self, int intname, int flags)
{
int ndx = LookupMove(self->GetClass(), FName(ENamedName(intname)));
if (ndx < 0 || ndx >= moves.SSize()) ThrowAbortException(X_ARRAY_OUT_OF_BOUNDS, "Bad 'move' index");
self->curMove = &moves[ndx];
self->spr.hitag = flags;
self->counter = 0;
Expand All @@ -766,10 +768,13 @@ DEFINE_ACTION_FUNCTION_NATIVE(DDukeActor, SetMove, Duke_SetMove)
void Duke_SetAI(DDukeActor* self, int intname)
{
int ndx = LookupAI(self->GetClass(), FName(ENamedName(intname)));
if (ndx < 0 || ndx >= ais.SSize()) ThrowAbortException(X_ARRAY_OUT_OF_BOUNDS, "Bad 'ai' index");
auto ai = &ais[ndx];
assert(!(ai->move & 0x80000000));
assert(!(ai->action & 0x80000000));
if (ai->move < 0 || ai->move >= moves.SSize()) ThrowAbortException(X_ARRAY_OUT_OF_BOUNDS, "Bad 'move' index");
self->curMove = &moves[ai->move];
if (ai->action < 0 || ai->action >= actions.SSize()) ThrowAbortException(X_ARRAY_OUT_OF_BOUNDS, "Bad 'action' index");
self->curAction = &actions[ai->action];
self->spr.hitag = ai->moveflags;
self->curAI = ai->name;
Expand Down

0 comments on commit 1dedb43

Please sign in to comment.