Fixed the return values of a hook getting corrupted if another hook was called during the execution of the current hook (from Mr.Stalin)

Added a fix to prevent hook scripts from being executed when the depth limit is exceeded, or the hook is called recursively.

sfall/Modules/HookScripts/Common.cpp

@@ -8,18 +8,24 @@ namespace sfall
 constexpr int maxArgs = 16;
 constexpr int maxDepth = 8;
 
-DWORD args[maxArgs]; // current hook arguments
+struct {
-DWORD oldargs[maxArgs * maxDepth];
+	DWORD hookID;
-DWORD* argPtr;
+	DWORD argCount;
-DWORD rets[16]; // current hook return values
+	DWORD cArg;
+	DWORD cRet;
+	DWORD oldArgs[maxArgs];
+	DWORD oldRets[maxRets];
+} savedArgs[maxDepth];
 
-DWORD firstArg = 0;
+static DWORD callDepth;
-DWORD callDepth;
+static DWORD currentRunHook = -1;
-DWORD lastCount[maxDepth];
+
+DWORD args[maxArgs]; // current hook arguments
+DWORD rets[maxRets]; // current hook return values
 
 DWORD argCount;
-DWORD cArg; // how many arguments were taken by current hook script
+DWORD cArg;    // how many arguments were taken by current hook script
-DWORD cRet; // how many return values were set by current hook script
+DWORD cRet;    // how many return values were set by current hook script
 DWORD cRetTmp; // how many return values were set by specific hook script (when using register_hook)
 
 std::vector<HookScript> hooks[numHooks];
@@ -52,19 +58,25 @@ bool LoadHookScript(const char* name, int id) {
 }
 
 void _stdcall BeginHook() {
-	if (callDepth <= maxDepth) {
+	if (callDepth && callDepth <= maxDepth) {
-		if (callDepth) {
+		// save all values of the current hook if another hook was called during the execution of the current hook
-			lastCount[callDepth - 1] = argCount;
+		int cDepth = callDepth - 1;
-			memcpy(&oldargs[maxArgs * (callDepth - 1)], args, maxArgs * sizeof(DWORD));
+		savedArgs[cDepth].hookID = currentRunHook;
-		}
+		savedArgs[cDepth].argCount = argCount;                                     // number of arguments of the current hook
-		argPtr = args;
+		savedArgs[cDepth].cArg = cArg;                                             // current count of taken arguments
-		for (DWORD i = 0; i < callDepth; i++) {
+		savedArgs[cDepth].cRet = cRet;                                             // number of return values for the current hook
-			argPtr += lastCount[i];
+		memcpy(&savedArgs[cDepth].oldArgs, args, argCount * sizeof(DWORD));        // values of the arguments
-		}
+		if (cRet) memcpy(&savedArgs[cDepth].oldRets, rets, cRet * sizeof(DWORD));  // return values
+
+		// for debugging
+		/*dlog_f("Saved cArgs/cRet: %d / %d(%d)\n", DL_HOOK, savedArgs[cDepth].argCount, savedArgs[cDepth].cRet, cRetTmp);
+		for (unsigned int i = 0; i < maxArgs; i++) {
+			dlog_f("Saved Args/Rets: %d / %d\n", DL_HOOK, savedArgs[cDepth].oldArgs[i], ((i < maxRets) ? savedArgs[cDepth].oldRets[i] : -1));
+		}*/
 	}
 	callDepth++;
 	#ifndef NDEBUG
-		dlog_f("Begin running hook, current depth: %d\n", DL_HOOK, callDepth);
+		dlog_f("Begin running hook, current depth: %d, current executable hook: %d\n", DL_HOOK, callDepth, currentRunHook);
 	#endif
 }
 
@@ -81,7 +93,15 @@ static void _stdcall RunSpecificHookScript(HookScript *hook) {
 void _stdcall RunHookScript(DWORD hook) {
 	cRet = 0;
 	if (hooks[hook].size()) {
-		dlog_f("Running hook %d, which has %0d entries attached\n", DL_HOOK, hook, hooks[hook].size());
+		if (callDepth > 1) {
+			if (hook == currentRunHook || callDepth > 8) {
+				fo::func::debug_printf("\n[SFALL] The hook ID: %d cannot be executed.", hook);
+				dlog_f("The hook %d cannot be executed due to exceeded depth limit or recursive calls\n", DL_MAIN, hook);
+				return;
+			}
+		}
+		currentRunHook = hook;
+		dlog_f("Running hook %d, which has %0d entries attached, depth: %d\n", DL_HOOK, hook, hooks[hook].size(), callDepth);
 		for (int i = hooks[hook].size() - 1; i >= 0; i--) {
 			RunSpecificHookScript(&hooks[hook][i]);
 		}
@@ -92,12 +112,24 @@ void _stdcall RunHookScript(DWORD hook) {
 
 void _stdcall EndHook() {
 	#ifndef NDEBUG
-		dlog_f("End running hook, current depth: %d\n", DL_HOOK, callDepth);
+		dlog_f("End running hook %d, current depth: %d\n", DL_HOOK, currentRunHook, callDepth);
 	#endif
 	callDepth--;
 	if (callDepth && callDepth <= maxDepth) {
-		argCount = lastCount[callDepth - 1];
+		// restore all saved values of the previous hook
-		memcpy(args, &oldargs[maxArgs * (callDepth - 1)], maxArgs * sizeof(DWORD));
+		int cDepth = callDepth - 1;
+		currentRunHook = savedArgs[cDepth].hookID;
+		argCount = savedArgs[cDepth].argCount;
+		cArg = savedArgs[cDepth].cArg;
+		cRet = cRetTmp = savedArgs[cDepth].cRet;  // also restore current count of the number of return values
+		memcpy(args, &savedArgs[cDepth].oldArgs, argCount * sizeof(DWORD));
+		if (cRet) memcpy(rets, &savedArgs[cDepth].oldRets, cRet * sizeof(DWORD));
+
+		// for debugging
+		/*dlog_f("Restored cArgs/cRets: %d / %d(%d)\n", DL_HOOK, argCount, cRet, cRetTmp);
+		for (unsigned int i = 0; i < maxArgs; i++) {
+			dlog_f("Restored Args/Rets: %d / %d\n", args[i], ((i < maxRets) ? rets[i] : -1));
+		}*/
 	}
 }
 

sfall/Modules/HookScripts/Common.h

@@ -19,25 +19,19 @@ const int maxRets = 8;
 // Struct for registered hook script
 struct HookScript {
 	ScriptProgram prog;
-	int callback; // proc number in script's proc table
+	int callback;        // proc number in script's proc table
 	bool isGlobalScript; // false for hs_* scripts, true for gl* scripts
 };
 
 // All currently registered hook scripts
 extern std::vector<HookScript> hooks[];
 
-extern DWORD args[]; // current hook arguments
+extern DWORD args[];  // current hook arguments
-extern DWORD oldargs[];
+extern DWORD rets[];  // current hook return values
-extern DWORD* argPtr;
-extern DWORD rets[]; // current hook return values
-
-extern DWORD firstArg;
-extern DWORD callDepth;
-extern DWORD lastCount[];
 
 extern DWORD argCount;
-extern DWORD cArg; // how many arguments were taken by current hook script
+extern DWORD cArg;    // how many arguments were taken by current hook script
-extern DWORD cRet; // how many return values were set by current hook script
+extern DWORD cRet;    // how many return values were set by current hook script
 extern DWORD cRetTmp; // how many return values were set by specific hook script (when using register_hook)
 
 bool LoadHookScript(const char* name, int id);

sfall/Modules/QuestList.cpp

@@ -347,7 +347,7 @@ skip:
 }
 
 static void RegisterButtonSoundFunc0() {
-	__asm { 
+	__asm {
 		mov  ebx, fo::funcoffs::gsound_red_butt_release_;
 		mov  edx, fo::funcoffs::gsound_red_butt_press_;
 		call fo::funcoffs::win_register_button_sound_func_;
@@ -392,7 +392,7 @@ static void __declspec(naked) StartPipboy_hack() {
 	if (questsButtonsType > 1) {
 		height = 23;
 		width = 22;
-		
+
 		indexUpArt0   = 54;
 		indexDownArt0 = 52;
 		indexUpArt1   = 53;
@@ -400,7 +400,7 @@ static void __declspec(naked) StartPipboy_hack() {
 	} else {
 		height = 14;
 		width = 11;
-		
+
 		indexUpArt0   = 181;
 		indexDownArt0 = 182;
 		indexUpArt1   = 199;
@@ -422,11 +422,11 @@ static void __declspec(naked) StartPipboy_hack() {
 
 	// creating new 2 buttons
 	picDown = (BYTE*)fo::var::optionsButtonDown1;
-	picUp   = (BYTE*)fo::var::optionsButtonUp1;	
+	picUp   = (BYTE*)fo::var::optionsButtonUp1;
 	if (fo::func::win_register_button(winRef, xPos, yPos, width, height, -1, -1, -1, 0x300, picUp,  picDown, 0, 32) != -1) {
 		RegisterButtonSoundFunc0();
 	}
-	
+
 	picDown = (BYTE*)fo::var::optionsButtonDown;
 	picUp   = (BYTE*)fo::var::optionsButtonUp;
 	if (fo::func::win_register_button(winRef, xPos, yPos + height, width, height, -1, -1, -1, 0x301, picUp,  picDown, 0, 32) != -1) {