From b2729a515607f1b0108d38b816430797f558c57f Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Sun, 16 Mar 2014 11:26:30 +0100 Subject: Fix bounds check in String::remove() Previously, if you passed in a very big index and/or count, the `index + count` could overflow, making the count be used as-is instead of being truncated (causing the string to be updated wrongly and potentially writing to arbitrary memory locations). We can rewrite the comparison to use `len - index` instead. Since we know that index < len, we are sure this subtraction does not overflow, regardless of what values of index and count we pass in. As an added bonus, the `len - index` value already needed be calculated inside the if, so this saves a few instructions in the generated code. To illustrate this problem, consider this code: String foo = "foo"; Serial.println(foo.length()); // Prints 3 foo.remove(1, 65535); // Should remove all but first character Serial.println(foo.length()); // Prints 4 without this patch Not shown in this is example is that some arbitrary memory is written as well. --- cores/arduino/WString.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cores/arduino/WString.cpp') diff --git a/cores/arduino/WString.cpp b/cores/arduino/WString.cpp index a0a3b6c..e4bed03 100644 --- a/cores/arduino/WString.cpp +++ b/cores/arduino/WString.cpp @@ -691,7 +691,7 @@ void String::remove(unsigned int index){ void String::remove(unsigned int index, unsigned int count){ if (index >= len) { return; } if (count <= 0) { return; } - if (index + count > len) { count = len - index; } + if (count > len - index) { count = len - index; } char *writeTo = buffer + index; len = len - count; strncpy(writeTo, buffer + index + count,len - index); -- cgit v1.2.3-18-g5258