SCI: Use strnlen instead of strlen to avoid buffer overflows

3 files changed, 17 insertions, 13 deletions

diff --git a/engines/sci/engine/kfile.cpp b/engines/sci/engine/kfile.cpp
index 49ad4caedb..25483b6507 100644
--- a/engines/sci/engine/kfile.cpp
+++ b/engines/sci/engine/kfile.cpp
@@ -338,7 +338,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 				score = Common::String::format("%u%03u", save.highScore, save.lowScore);
 			}
 
-			const uint nameLength = strlen(save.name);
+			const uint nameLength = Common::strnlen(save.name, SCI_MAX_SAVENAME_LENGTH);
 			const uint size = nameLength + /* \r\n */ 2 + score.size();
 			char *buffer = (char *)malloc(size);
 			memcpy(buffer, save.name, nameLength);
@@ -372,7 +372,7 @@ reg_t kFileIOOpen(EngineState *s, int argc, reg_t *argv) {
 			fillSavegameDesc(g_sci->getSavegameName(saveNo), &save);
 
 			const Common::String avatarId = Common::String::format("%02d", save.avatarId);
-			const uint nameLength = strlen(save.name);
+			const uint nameLength = Common::strnlen(save.name, SCI_MAX_SAVENAME_LENGTH);
 			const uint size = nameLength + /* \r\n */ 2 + avatarId.size() + 1;
 			char *buffer = (char *)malloc(size);
 			memcpy(buffer, save.name, nameLength);
diff --git a/engines/sci/engine/seg_manager.cpp b/engines/sci/engine/seg_manager.cpp
index 23d1942dd3..f8138ef649 100644
--- a/engines/sci/engine/seg_manager.cpp
+++ b/engines/sci/engine/seg_manager.cpp
@@ -786,7 +786,7 @@ size_t SegManager::strlen(reg_t str) {
 	}
 
 	if (str_r.isRaw) {
-		return ::strlen((const char *)str_r.raw);
+		return Common::strnlen((const char *)str_r.raw, str_r.maxSize);
 	} else {
 		int i = 0;
 		while (getChar(str_r, i))
@@ -807,19 +807,23 @@ Common::String SegManager::getString(reg_t pointer) {
 		return ret;
 	}
 
-	if (src_r.isRaw)
-		ret = (char *)src_r.raw;
-	else {
+	if (src_r.isRaw) {
+		// There is no guarantee that raw strings are zero-terminated; for
+		// example, Phant1 reads "\r\n" from a pointer of size 2 during the
+		// chase
+		const uint size = Common::strnlen((const char *)src_r.raw, src_r.maxSize);
+		ret = Common::String((const char *)src_r.raw, size);
+	} else {
 		uint i = 0;
-		for (;;) {
-			char c = getChar(src_r, i);
+		while (i < (uint)src_r.maxSize) {
+			const char c = getChar(src_r, i);
 
 			if (!c)
 				break;
 
 			i++;
 			ret += c;
-		};
+		}
 	}
 	return ret;
 }
diff --git a/engines/sci/parser/vocabulary.cpp b/engines/sci/parser/vocabulary.cpp
index a0f958167d..67197fc29f 100644
--- a/engines/sci/parser/vocabulary.cpp
+++ b/engines/sci/parser/vocabulary.cpp
@@ -208,7 +208,7 @@ bool Vocabulary::loadSuffixes() {
 		suffix_t suffix;
 
 		suffix.alt_suffix = (const char *)resource->data + seeker;
-		suffix.alt_suffix_length = strlen(suffix.alt_suffix);
+		suffix.alt_suffix_length = Common::strnlen(suffix.alt_suffix, resource->size - seeker);
 		seeker += suffix.alt_suffix_length + 1; // Hit end of string
 
 		suffix.result_class = (int16)READ_BE_UINT16(resource->data + seeker);
@@ -218,7 +218,7 @@ bool Vocabulary::loadSuffixes() {
 		seeker++;
 
 		suffix.word_suffix = (const char *)resource->data + seeker;
-		suffix.word_suffix_length = strlen(suffix.word_suffix);
+		suffix.word_suffix_length = Common::strnlen(suffix.word_suffix, resource->size - seeker);
 		seeker += suffix.word_suffix_length + 1;
 
 		suffix.class_mask = (int16)READ_BE_UINT16(resource->data + seeker);
@@ -288,12 +288,12 @@ bool Vocabulary::loadAltInputs() {
 		AltInput t;
 		t._input = data;
 
-		uint32 l = strlen(data);
+		uint32 l = Common::strnlen(data, data_end - data);
 		t._inputLength = l;
 		data += l + 1;
 
 		t._replacement = data;
-		l = strlen(data);
+		l = Common::strnlen(data, data_end - data);
 		data += l + 1;
 
 		if (data < data_end && strncmp(data, t._input, t._inputLength) == 0)