Skip to contentSkip to conversation

Fix buffer overflow in print_result() function

PATCH New [email protected] 2 participants 2 messages Nov 7, 2017 – Nov 8, 2017 Library code

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 Fix buffer overflow in print_result() function

From: Veronika Kabatova <[email protected]>

From man page: "The functions snprintf() and vsnprintf() do not write more than size bytes (including the terminating null byte ('\0')). If the output was truncated due to this limit, then the return value is the number of characters (excluding the terminating null byte) which would have been written to the final string if enough space had been available. Thus, a return value of size or more means that the output was truncated."

The return value is not checked and blindly subtracted from available space in the buffer which may cause negative values (which are then converted to unsigned type size_t) in case the variables to print don't fit into buffer. The pointer to buffer is also moved forward the same amount, and writing into memory that is not allocated causes unexpected failures of tests with segfaults.

This was discovered during rewriting of mtest06/mmap1 testcase to new library where verbose output tried to print out memory contents longer than space available in the buffer. The framework should be robust enough to handle these situations.

---
 lib/tst_test.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)
lib/tst_test.c +11 -6
index c8baf2a43..b43fb35f7 100644
@@ -227,13 +227,18 @@ static void print_result(const char *file, const int lineno, int ttype,
 	size -= ret;
 
 	ret = vsnprintf(str, size, fmt, va);
-	str += ret;
-	size -= ret;
-
-	if (str_errno) {
+	str += MIN(ret, size - 2);
+	size -= MIN(ret, size - 2);
+	if (ret >= size - 2) {
+		tst_res_(file, lineno, TWARN,
+				"Next message is too long and truncated:");
+	} else if (str_errno) {
 		ret = snprintf(str, size, ": %s", str_errno);
-		str += ret;
-		size -= ret;
+		str += MIN(ret, size - 2);
+		size -= MIN(ret, size - 2);
+		if (ret >= size - 2)
+			tst_res_(file, lineno, TWARN,
+				"Next message is too long and truncated:");
 	}
 
 	snprintf(str, size, "\n");
Signature · 1 line
2.13.6
#1 lore raw

PATCH v2 1/1 Fix buffer overflow in print_result() function

Hi!

Quoted 5 lines
lib/tst_test.c +0 -0
index c8baf2a43..b43fb35f7 100644

lib/tst_test.c static void print_result(const char *file, const int lineno, int ttype, · line 232

Quoted 10 lines
lib/tst_test.c +3 -4
@@ -227,13 +227,18 @@ static void print_result(const char *file, const int lineno, int ttype,
 	size -= ret;
 
 	ret = vsnprintf(str, size, fmt, va);
-	str += ret;
-	size -= ret;
-
-	if (str_errno) {
+	str += MIN(ret, size - 2);
+	size -= MIN(ret, size - 2);
+	if (ret >= size - 2) {

We modify the size before this condition, so the warning was triggered even for string that were half of the size of the buffer.

So I've changed the code to save the size-2 into a variable before we modify it so that we can use it in the condition.

lib/tst_test.c static void print_result(const char *file, const int lineno, int ttype, · line 241

Quoted 11 lines
lib/tst_test.c +8 -2
+		tst_res_(file, lineno, TWARN,
+				"Next message is too long and truncated:");
+	} else if (str_errno) {
 		ret = snprintf(str, size, ": %s", str_errno);
-		str += ret;
-		size -= ret;
+		str += MIN(ret, size - 2);
+		size -= MIN(ret, size - 2);
+		if (ret >= size - 2)
+			tst_res_(file, lineno, TWARN,
+				"Next message is too long and truncated:");

And here as well.

I've also added a testcase and pushed, thanks.

Signature · 2 lines
Cyril Hrubis
[email protected]