Added bounds check for local variables in ACS VM

src/p_acs.cpp

@@ -644,7 +644,7 @@ inline int PitchToACS(DAngle ang)
 
 struct CallReturn
 {
-	CallReturn(int pc, ScriptFunction *func, FBehavior *module, int32_t *locals, ACSLocalArrays *arrays, bool discard, unsigned int runaway)
+	CallReturn(int pc, ScriptFunction *func, FBehavior *module, const ACSLocalVariables &locals, ACSLocalArrays *arrays, bool discard, unsigned int runaway)
 		: ReturnFunction(func),
 		  ReturnModule(module),
 		  ReturnLocals(locals),
@@ -656,7 +656,7 @@ struct CallReturn
 
 	ScriptFunction *ReturnFunction;
 	FBehavior *ReturnModule;
-	int32_t *ReturnLocals;
+	ACSLocalVariables ReturnLocals;
 	ACSLocalArrays *ReturnArrays;
 	int ReturnAddress;
 	int bDiscardResult;
@@ -6967,7 +6967,7 @@ static void SetMarineSprite(AActor *marine, PClassActor *source)
 int DLevelScript::RunScript ()
 {
 	DACSThinker *controller = DACSThinker::ActiveThinker;
-	int32_t *locals = &Localvars[0];
+	ACSLocalVariables locals(Localvars);
 	ACSLocalArrays noarrays;
 	ACSLocalArrays *localarrays = &noarrays;
 	ScriptFunction *activeFunction = NULL;
@@ -7335,7 +7335,6 @@ int DLevelScript::RunScript ()
 				int i;
 				ScriptFunction *func;
 				FBehavior *module;
-				int32_t *mylocals;
 
 				if(pcd == PCD_CALLSTACK)
 				{
@@ -7364,9 +7363,9 @@ int DLevelScript::RunScript ()
 					state = SCRIPT_PleaseRemove;
 					break;
 				}
-				mylocals = locals;
+				const ACSLocalVariables mylocals = locals;
 				// The function's first argument is also its first local variable.
-				locals = &Stack[sp - func->ArgCount];
+				locals.Reset(&Stack[sp - func->ArgCount], func->ArgCount + func->LocalCount);
 				// Make space on the stack for any other variables the function uses.
 				for (i = 0; i < func->LocalCount; ++i)
 				{
@@ -7405,7 +7404,7 @@ int DLevelScript::RunScript ()
 				sp -= sizeof(CallReturn)/sizeof(int);
 				retsp = &Stack[sp];
 				activeBehavior->GetFunctionProfileData(activeFunction)->AddRun(runaway - ret->EntryInstrCount);
-				sp = int(locals - &Stack[0]);
+				sp = int(locals.GetPointer() - &Stack[0]);
 				pc = ret->ReturnModule->Ofs2PC(ret->ReturnAddress);
 				activeFunction = ret->ReturnFunction;
 				activeBehavior = ret->ReturnModule;

src/p_acs.h

@@ -147,6 +147,44 @@ struct ProfileCollector
 	int Index;
 };
 
+class ACSLocalVariables
+{
+public:
+	ACSLocalVariables(TArray<int32_t> &variables)
+	: memory(&variables[0])
+	, count(variables.Size())
+	{
+	}
+
+	void Reset(int32_t *const memory, const size_t count)
+	{
+		// TODO: pointer sanity check?
+		// TODO: constraints on count?
+
+		this->memory = memory;
+		this->count = count;
+	}
+
+	int32_t& operator[](const size_t index)
+	{
+		if (index >= count)
+		{
+			I_Error("Out of bounds access to local variables in ACS VM");
+		}
+
+		return memory[index];
+	}
+
+	const int32_t *GetPointer() const
+	{
+		return memory;
+	}
+
+private:
+	int32_t *memory;
+	size_t count;
+};
+
 struct ACSLocalArrayInfo
 {
 	unsigned int Size;
@@ -173,7 +211,7 @@ struct ACSLocalArrays
 	}
 
 	// Bounds-checking Set and Get for local arrays
-	void Set(int *locals, int arraynum, int arrayentry, int value)
+	void Set(ACSLocalVariables &locals, int arraynum, int arrayentry, int value)
 	{
 		if ((unsigned int)arraynum < Count &&
 			(unsigned int)arrayentry < Info[arraynum].Size)
@@ -181,7 +219,7 @@ struct ACSLocalArrays
 			locals[Info[arraynum].Offset + arrayentry] = value;
 		}
 	}
-	int Get(int *locals, int arraynum, int arrayentry)
+	int Get(ACSLocalVariables &locals, int arraynum, int arrayentry)
 	{
 		if ((unsigned int)arraynum < Count &&
 			(unsigned int)arrayentry < Info[arraynum].Size)