ref: 06821e944818e7f064aeb95cbdef5f12dd85a729
parent: 5eeef20d70fc978372058371d29821dc27b019a5
author: Simon Howard <fraggle@soulsphere.org>
date: Thu Nov 27 11:54:05 EST 2014
hexen: audit calls to P_StartACS(). Scripts can have a maximum of three arguments when started (see ACS_Execute / #80 linedef special type). This means that the array of arguments passed must be at least three elements in length. When loading scripts, check the argument count never exceeds 3, and print a warning message if it does. Don't pass pointers to structure members implicitly treating them as elements of an array (as this relies on compiler-dependent behavior as to how structure members are laid out). Instead, copy values into a temporary array to make the intended behavior explicit. This fixes #477. Thanks to Quasar for reporting it.
--- a/src/hexen/p_acs.c
+++ b/src/hexen/p_acs.c
@@ -27,6 +27,7 @@
// MACROS ------------------------------------------------------------------
+#define MAX_SCRIPT_ARGS 3
#define SCRIPT_CONTINUE 0
#define SCRIPT_STOP 1
#define SCRIPT_TERMINATE 2
@@ -195,7 +196,7 @@
static int (*PCodeCmds[]) (void) =
{-CmdNOP,
+ CmdNOP,
CmdTerminate,
CmdSuspend,
CmdPushNumber,
@@ -294,7 +295,10 @@
CmdSoundSequence,
CmdSetLineTexture,
CmdSetLineBlocking,
- CmdSetLineSpecial, CmdThingSound, CmdEndPrintBold};
+ CmdSetLineSpecial,
+ CmdThingSound,
+ CmdEndPrintBold,
+};
// CODE --------------------------------------------------------------------
@@ -336,6 +340,16 @@
info->argCount = LONG(*buffer);
++buffer;
+ if (info->argCount > MAX_SCRIPT_ARGS)
+ {+ fprintf(stderr, "Warning: ACS script #%i has %i arguments, more "
+ "than the maximum of %i. Enforcing limit.\n"
+ "If you are seeing this message, please report "
+ "the name of the WAD where you saw it.\n",
+ i, info->argCount, MAX_SCRIPT_ARGS);
+ info->argCount = MAX_SCRIPT_ARGS;
+ }
+
if (info->number >= OPEN_SCRIPTS_BASE)
{ // Auto-activateinfo->number -= OPEN_SCRIPTS_BASE;
@@ -414,6 +428,9 @@
//
// P_StartACS
//
+// Start an ACS script. The 'args' array should be at least MAX_SCRIPT_ARGS
+// elements in length.
+//
//==========================================================================
static char ErrorMsg[128];
@@ -458,7 +475,7 @@
script->side = side;
script->ip = ACSInfo[infoIndex].address;
script->thinker.function = T_InterpretACS;
- for (i = 0; i < ACSInfo[infoIndex].argCount; i++)
+ for (i = 0; i < MAX_SCRIPT_ARGS && i < ACSInfo[infoIndex].argCount; i++)
{script->vars[i] = args[i];
}
@@ -503,7 +520,7 @@
}
ACSStore[index].map = map;
ACSStore[index].script = number;
- memcpy(ACSStore[index].args, args, sizeof(int));
+ memcpy(ACSStore[index].args, args, MAX_SCRIPT_ARGS);
return true;
}
--- a/src/hexen/p_enemy.c
+++ b/src/hexen/p_enemy.c
@@ -4781,7 +4781,8 @@
}
}
else if (actor->flags & MF_COUNTKILL && actor->special)
- { // Initiate monster death actions+ {+ // Initiate monster death actions.
P_ExecuteLineSpecial(actor->special, actor->args, NULL, 0, actor);
}
}
@@ -4933,7 +4934,7 @@
{mobj_t *spot;
int lastfound;
- byte args[3] = { 0, 0, 0 };+ byte args[3] = {0, 0, 0};if ((!actor->special2.i) &&
(actor->health <= (actor->info->spawnhealth / 2)))
--- a/src/hexen/p_inter.c
+++ b/src/hexen/p_inter.c
@@ -1280,7 +1280,7 @@
void P_KillMobj(mobj_t * source, mobj_t * target)
{- int dummy;
+ byte dummyArgs[3] = {0, 0, 0};mobj_t *master;
target->flags &= ~(MF_SHOOTABLE | MF_FLOAT | MF_SKULLFLY | MF_NOGRAVITY);
@@ -1292,8 +1292,7 @@
{ // Initiate monster death actionsif (target->type == MT_SORCBOSS)
{- dummy = 0;
- P_StartACS(target->special, 0, (byte *) & dummy, target, NULL, 0);
+ P_StartACS(target->special, 0, dummyArgs, target, NULL, 0);
}
else
{--- a/src/hexen/p_map.c
+++ b/src/hexen/p_map.c
@@ -2003,6 +2003,7 @@
boolean PTR_PuzzleItemTraverse(intercept_t * in)
{mobj_t *mobj;
+ byte args[3];
int sound;
if (in->isaline)
@@ -2045,8 +2046,14 @@
{ // Item type doesn't matchreturn false;
}
- P_StartACS(in->d.line->arg2, 0, &in->d.line->arg3,
- PuzzleItemUser, in->d.line, 0);
+
+ // Construct an args[] array that would contain the values from
+ // the line that would be passed by Vanilla Hexen.
+ args[0] = in->d.line->arg3;
+ args[1] = in->d.line->arg4;
+ args[2] = in->d.line->arg5;
+
+ P_StartACS(in->d.line->arg2, 0, args, PuzzleItemUser, in->d.line, 0);
in->d.line->special = 0;
PuzzleActivated = true;
return false; // Stop searching
@@ -2061,6 +2068,7 @@
{ // Item type doesn't matchreturn true;
}
+
P_StartACS(mobj->args[1], 0, &mobj->args[2], PuzzleItemUser, NULL, 0);
mobj->special = 0;
PuzzleActivated = true;
--- a/src/hexen/p_spec.c
+++ b/src/hexen/p_spec.c
@@ -498,6 +498,9 @@
//
// P_ExecuteLineSpecial
//
+// Invoked when crossing a linedef. The args[] array should be at least
+// 5 elements in length.
+//
//============================================================================
boolean P_ExecuteLineSpecial(int special, byte * args, line_t * line,
@@ -842,6 +845,7 @@
boolean P_ActivateLine(line_t * line, mobj_t * mo, int side,
int activationType)
{+ byte args[5];
int lineActivation;
boolean repeat;
boolean buttonSuccess;
@@ -863,8 +867,14 @@
repeat = line->flags & ML_REPEAT_SPECIAL;
buttonSuccess = false;
- buttonSuccess = P_ExecuteLineSpecial(line->special, &line->arg1, line,
- side, mo);
+ // Construct args[] array to contain the arguments from the line, as we
+ // cannot rely on struct field ordering and layout.
+ args[0] = line->arg1;
+ args[1] = line->arg2;
+ args[2] = line->arg3;
+ args[3] = line->arg4;
+ args[4] = line->arg5;
+ buttonSuccess = P_ExecuteLineSpecial(line->special, args, line, side, mo);
if (!repeat && buttonSuccess)
{ // clear the special on non-retriggerable linesline->special = 0;