[Scummvm-git-logs] scummvm master -> c919c9996c6f62cf4f0d1a22d0522b0ee9a0514c

[wjp]

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

Summary:
9d3893459f COMMON: Add strnlen for safer C string length reads
c919c9996c TESTS: Fix warnings


Commit: 9d3893459f34b6ada2dad2d9d27216c774a7c4bd
    https://github.com/scummvm/scummvm/commit/9d3893459f34b6ada2dad2d9d27216c774a7c4bd
Author: Colin Snover (github.com at zetafleet.com)
Date: 2017-01-05T22:07:24+01:00

Commit Message:
COMMON: Add strnlen for safer C string length reads

This API is intended for use in cases where C strings come
from untrusted sources like game files, where malformed data
missing the null terminator would cause strlen to read out of
bounds.

Changed paths:
    common/str.cpp
    common/str.h
    test/common/str.h


diff --git a/common/str.cpp b/common/str.cpp
index 90bd539..3a0fd6a 100644
--- a/common/str.cpp
+++ b/common/str.cpp
@@ -942,6 +942,13 @@ size_t strlcat(char *dst, const char *src, size_t size) {
 	return dstLength + (src - srcStart);
 }
 
+size_t strnlen(const char *src, size_t maxSize) {
+	size_t counter = 0;
+	while (counter != maxSize && *src++)
+		++counter;
+	return counter;
+}
+
 } // End of namespace Common
 
 // Portable implementation of stricmp / strcasecmp / strcmpi.
diff --git a/common/str.h b/common/str.h
index d55ba07..ba1e0b8 100644
--- a/common/str.h
+++ b/common/str.h
@@ -445,6 +445,17 @@ size_t strlcpy(char *dst, const char *src, size_t size);
 size_t strlcat(char *dst, const char *src, size_t size);
 
 /**
+ * Determine the length of a string up to a maximum of `maxSize` characters.
+ * This should be used instead of `strlen` when reading the length of a C string
+ * from potentially unsafe or corrupt sources, like game assets.
+ *
+ * @param src The source string.
+ * @param maxSize The maximum size of the string.
+ * @return The length of the string.
+ */
+size_t strnlen(const char *src, size_t maxSize);
+
+/**
  * Convenience wrapper for tag2string which "returns" a C string.
  * Note: It is *NOT* safe to do anything with the return value other than directly
  * copying or printing it.
diff --git a/test/common/str.h b/test/common/str.h
index c59c5a5..b6080fe 100644
--- a/test/common/str.h
+++ b/test/common/str.h
@@ -403,6 +403,29 @@ class StringTestSuite : public CxxTest::TestSuite
 		TS_ASSERT_EQUALS(strcmp(test4, resultString), 0);
 	}
 
+	void test_strnlen() {
+		static const char * const testString = "123";
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 0), 0);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 1), 1);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 2), 2);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 3), 3);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 4), 3);
+
+		const char testArray[4] = { '1', '2', '3', '4' };
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 0), 0);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 1), 1);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 2), 2);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 3), 3);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 4), 4);
+
+		const char testArray2[4] = { '1', '\0', '3', '4' };
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 0), 0);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 1), 1);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 2), 1);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 3), 1);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 4), 1);
+	}
+
 	void test_scumm_stricmp() {
 		TS_ASSERT_EQUALS(scumm_stricmp("abCd", "abCd"), 0);
 		TS_ASSERT_EQUALS(scumm_stricmp("abCd", "ABCd"), 0);


Commit: c919c9996c6f62cf4f0d1a22d0522b0ee9a0514c
    https://github.com/scummvm/scummvm/commit/c919c9996c6f62cf4f0d1a22d0522b0ee9a0514c
Author: Willem Jan Palenstijn (wjp at usecode.org)
Date: 2017-01-05T22:07:24+01:00

Commit Message:
TESTS: Fix warnings

Changed paths:
    test/common/str.h


diff --git a/test/common/str.h b/test/common/str.h
index b6080fe..b7ad28e 100644
--- a/test/common/str.h
+++ b/test/common/str.h
@@ -405,25 +405,25 @@ class StringTestSuite : public CxxTest::TestSuite
 
 	void test_strnlen() {
 		static const char * const testString = "123";
-		TS_ASSERT_EQUALS(Common::strnlen(testString, 0), 0);
-		TS_ASSERT_EQUALS(Common::strnlen(testString, 1), 1);
-		TS_ASSERT_EQUALS(Common::strnlen(testString, 2), 2);
-		TS_ASSERT_EQUALS(Common::strnlen(testString, 3), 3);
-		TS_ASSERT_EQUALS(Common::strnlen(testString, 4), 3);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 0), 0u);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 1), 1u);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 2), 2u);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 3), 3u);
+		TS_ASSERT_EQUALS(Common::strnlen(testString, 4), 3u);
 
 		const char testArray[4] = { '1', '2', '3', '4' };
-		TS_ASSERT_EQUALS(Common::strnlen(testArray, 0), 0);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray, 1), 1);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray, 2), 2);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray, 3), 3);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray, 4), 4);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 0), 0u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 1), 1u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 2), 2u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 3), 3u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray, 4), 4u);
 
 		const char testArray2[4] = { '1', '\0', '3', '4' };
-		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 0), 0);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 1), 1);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 2), 1);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 3), 1);
-		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 4), 1);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 0), 0u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 1), 1u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 2), 1u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 3), 1u);
+		TS_ASSERT_EQUALS(Common::strnlen(testArray2, 4), 1u);
 	}
 
 	void test_scumm_stricmp() {