[Scummvm-git-logs] scummvm master -> 31be7f6552930fee39f6bda396e47cce8d4f21af

[eriktorbjorn]

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

Summary:
31be7f6552 SCUMM: Relax bounds checking a bit in o5_stringOps()


Commit: 31be7f6552930fee39f6bda396e47cce8d4f21af
    https://github.com/scummvm/scummvm/commit/31be7f6552930fee39f6bda396e47cce8d4f21af
Author: Torbjörn Andersson (eriktorbjorn at users.sourceforge.net)
Date: 2025-05-02T19:27:06+02:00

Commit Message:
SCUMM: Relax bounds checking a bit in o5_stringOps()

Since ScummVM allocates a couple of exta bytes as a "safety area" for
each resource, allow strings to be read or written out of bounds as long
as they still stay in that safety area. I think scripts who do that are
buggy, but let's not break them as long as they're still well defined.

Changed paths:
    engines/scumm/resource.cpp
    engines/scumm/resource.h
    engines/scumm/script_v5.cpp


diff --git a/engines/scumm/resource.cpp b/engines/scumm/resource.cpp
index e630e6b601f..d80b14f4498 100644
--- a/engines/scumm/resource.cpp
+++ b/engines/scumm/resource.cpp
@@ -838,9 +838,6 @@ byte ResourceManager::Resource::getResourceCounter() const {
 	return _flags & RF_USAGE;
 }
 
-/* 2 bytes safety area to make "precaching" of bytes in the gdi drawer easier */
-#define SAFETY_AREA 2
-
 byte *ResourceManager::createResource(ResType type, ResId idx, uint32 size) {
 	debugC(DEBUG_RESOURCE, "_res->createResource(%s,%d,%d)", nameOfResType(type), idx, size);
 
diff --git a/engines/scumm/resource.h b/engines/scumm/resource.h
index 5df84e36820..c812c4a3922 100644
--- a/engines/scumm/resource.h
+++ b/engines/scumm/resource.h
@@ -27,6 +27,11 @@
 
 namespace Scumm {
 
+// Extra space allocated for every resource. Originaly a safety area to make
+// "precaching" of bytes in the gdi drawer easier. Now also taken into
+// consideration when doing bounds checking in o5_stringOps().
+#define SAFETY_AREA 2
+
 enum {
 	OF_OWNER_MASK = 0x0F,
 	OF_STATE_MASK = 0xF0,
diff --git a/engines/scumm/script_v5.cpp b/engines/scumm/script_v5.cpp
index e8e65d9168f..9854b240481 100644
--- a/engines/scumm/script_v5.cpp
+++ b/engines/scumm/script_v5.cpp
@@ -3060,6 +3060,10 @@ void ScummEngine_v5::o5_stringOps() {
 	// position. In that case we should technically return 48 (the ASCII
 	// code for "0"), but anything outside the 49-56 should be fine. See
 	// bug #15884 for further details.
+	//
+	// VGA Loom writes out of bounds on startup, but we allow it since it's
+	// still within the additional 2 bytes that gets allocated for each
+	// resources as a "safety area".
 
 	_opcode = fetchScriptByte();
 	switch (_opcode & 0x1F) {
@@ -3085,9 +3089,11 @@ void ScummEngine_v5::o5_stringOps() {
 		len = getResourceSize(rtString, a);
 		if (ptr == nullptr)
 			error("String %d does not exist", a);
-		if (b >= 0 && b < len)
+		if (b >= 0 && b < len + SAFETY_AREA) {
+			if (b >= len)
+				warning("o5_stringOps: Allowing writing %d to string %d (size %d) out of bounds (%d) since it's in the safety area", c, a, len, b);
 			ptr[b] = c;
-		else
+		} else
 			warning("o5_stringOps: Writing %d to string %d (size %d) out of bounds (%d)", c, a, len, b);
 		break;
 
@@ -3099,9 +3105,11 @@ void ScummEngine_v5::o5_stringOps() {
 		len = getResourceSize(rtString, a);
 		if (ptr == nullptr)
 			error("String %d does not exist", a);
-		if (b >= 0 && b < len)
+		if (b >= 0 && b < len + SAFETY_AREA) {
+			if (b >= len)
+				warning("o5_stringOps: Allowing reading from strings %d (size %d) out of bounds (%d) since it's in the safety earea", a, len, b);
 			setResult(ptr[b]);
-		else {
+		} else {
 			warning("o5_stringOps: Reading string %d (size %d) out of bounds (%d)", a, len, b);
 			setResult(0);
 		}