[Scummvm-git-logs] scummvm branch-2-5 -> bbaa4c42eaabd0c8c3a5b9de2b60c6ad69f5ee15

[athrxx]

This automated email contains information about 1 new commit which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
bbaa4c42ea KYRA: (LOL) - fix invalid script stack access


Commit: bbaa4c42eaabd0c8c3a5b9de2b60c6ad69f5ee15
    https://github.com/scummvm/scummvm/commit/bbaa4c42eaabd0c8c3a5b9de2b60c6ad69f5ee15
Author: athrxx (athrxx at scummvm.org)
Date: 2021-10-24T15:17:14+02:00

Commit Message:
KYRA: (LOL) - fix invalid script stack access

Thanks to benoit-pierre for finding this.

(see PR #3448, https://github.com/scummvm/scummvm/pull/3448)

I chose a different approach for the fix, since I wanted an easy way of detecting/preventing this for similiar cases and I also didn't want to have so much extra code in that particular script function.

Changed paths:
    engines/kyra/kyra_v1.h
    engines/kyra/script/script.h
    engines/kyra/script/script_lol.cpp


diff --git a/engines/kyra/kyra_v1.h b/engines/kyra/kyra_v1.h
index 591126db14..6c04e91338 100644
--- a/engines/kyra/kyra_v1.h
+++ b/engines/kyra/kyra_v1.h
@@ -300,6 +300,18 @@ protected:
 	int o1_blockOutWalkableRegion(EMCState *script);
 	int o1_playSoundEffect(EMCState *script);
 
+	// script debug
+#ifndef RELEASE_BUILD
+	int16 emcSafeReadStack(EMCState *s, int x, int line, const char *file) {
+		if (s->sp+x > EMCState::kStackLastEntry) {
+			//assert(sp+x < kStackSize);
+			warning("Invalid EMC stack read attempt from: '%s', line %d", file, line);
+			return 0;
+		}
+		return s->stack[s->sp+x];
+	}
+#endif
+
 	// items
 	int _mouseState;
 
diff --git a/engines/kyra/script/script.h b/engines/kyra/script/script.h
index 73f14b3f5c..7988a1d4de 100644
--- a/engines/kyra/script/script.h
+++ b/engines/kyra/script/script.h
@@ -60,7 +60,13 @@ struct EMCState {
 	int16 stack[kStackSize];  // VM stack
 };
 
+#ifdef RELEASE_BUILD
 #define stackPos(x) (script->stack[script->sp+x])
+#define safeStackPos(x) (script->sp+x < EMCState::kStackSize ? stackPos(x) : 0)
+#else
+#define stackPos(x) emcSafeReadStack(script, x, __LINE__, __FILE__)
+#define safeStackPos(x) stackPos(x)
+#endif
 #define stackPosString(x) ((const char *)&script->dataPtr->text[READ_BE_UINT16(&script->dataPtr->text[stackPos(x)<<1])])
 
 class Resource;
diff --git a/engines/kyra/script/script_lol.cpp b/engines/kyra/script/script_lol.cpp
index 15c8c365e4..a429f8eb73 100644
--- a/engines/kyra/script/script_lol.cpp
+++ b/engines/kyra/script/script_lol.cpp
@@ -1464,9 +1464,9 @@ int LoLEngine::olol_checkForCertainPartyMember(EMCState *script) {
 }
 
 int LoLEngine::olol_printMessage(EMCState *script) {
-	debugC(3, kDebugLevelScriptFuncs, "LoLEngine::olol_printMessage(%p) (%d, %d, %d, %d, %d, %d, %d, %d, %d, %d)", (const void *)script, stackPos(0), stackPos(1), stackPos(2), stackPos(3), stackPos(4), stackPos(5), stackPos(6), stackPos(7), stackPos(8), stackPos(9));
-	int snd = stackPos(2);
-	_txt->printMessage(stackPos(0), getLangString(stackPos(1)), stackPos(3), stackPos(4), stackPos(5), stackPos(6), stackPos(7), stackPos(8), stackPos(9));
+	debugC(3, kDebugLevelScriptFuncs, "LoLEngine::olol_printMessage(%p) (%d, %d, %d, %d, %d, %d, %d, %d, %d, %d)", (const void *)script, safeStackPos(0), safeStackPos(1), safeStackPos(2), safeStackPos(3), safeStackPos(4), safeStackPos(5), safeStackPos(6), safeStackPos(7), safeStackPos(8), safeStackPos(9));
+	int snd = safeStackPos(2);
+	_txt->printMessage(safeStackPos(0), getLangString(safeStackPos(1)), safeStackPos(3), safeStackPos(4), safeStackPos(5), safeStackPos(6), safeStackPos(7), safeStackPos(8), safeStackPos(9));
 
 	if (snd >= 0)
 		snd_playSoundEffect(snd, -1);