From 856f3ae6489b3f9aec62043c58c3961baf619f16 Mon Sep 17 00:00:00 2001
From: Colin Snover
Date: Fri, 18 Mar 2016 10:40:42 -0500
Subject: SCI32: More correctly fix kStringCopy overflow

This entire kString code needs to be reviewed/refactored, but
at least this fix is more complete than the last one.

Thanks to @lordhoto and @wjp for their assistance.
---
 engines/sci/engine/kstring.cpp | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

(limited to 'engines')

diff --git a/engines/sci/engine/kstring.cpp b/engines/sci/engine/kstring.cpp
index 6d61ad5f41..1c08bf597c 100644
--- a/engines/sci/engine/kstring.cpp
+++ b/engines/sci/engine/kstring.cpp
@@ -765,11 +765,14 @@ reg_t kStringCopy(EngineState *s, int argc, reg_t *argv) {
 	}
 
 	// The original engine ignores bad copies too
-	if (index2 > string2Size)
+	if (index2 >= string2Size)
 		return NULL_REG;
 
 	// A count of -1 means fill the rest of the array
-	uint32 count = argv[4].toSint16() == -1 ? string2Size - index2 + 1 : argv[4].toUint16();
+	uint32 count = string2Size - index2;
+	if (argv[4].toSint16() != -1) {
+		count = MIN(count, (uint32)argv[4].toUint16());
+	}
 //	reg_t strAddress = argv[0];
 
 	SciString *string1 = s->_segMan->lookupString(argv[0]);
@@ -781,8 +784,7 @@ reg_t kStringCopy(EngineState *s, int argc, reg_t *argv) {
 	// Note: We're accessing from c_str() here because the
 	// string's size ignores the trailing 0 and therefore
 	// triggers an assert when doing string2[i + index2].
-	uint16 size = MIN(string2Size, count);
-	for (uint16 i = 0; i < size; i++)
+	for (uint16 i = 0; i < count; i++)
 		string1->setValue(i + index1, string2[i + index2]);
 
 	return argv[0];
-- 
cgit v1.2.3