Make strcat() and strncat() more standard-compliant (check for invalid parameters even if zero bytes is copied, more accurate overlap check)
Fix the tests that were relying on the incorrect behavior.
git-svn-id: https://llvm.org/svn/llvm-project/compiler-rt/trunk@161167 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/asan/asan_interceptors.cc b/lib/asan/asan_interceptors.cc
index b0571f8..ec03fff 100644
--- a/lib/asan/asan_interceptors.cc
+++ b/lib/asan/asan_interceptors.cc
@@ -408,16 +408,22 @@
# endif
#endif // ASAN_INTERCEPT_INDEX
+// For both strcat() and strncat() we need to check the validity of |to|
+// argument irrespective of the |from| length.
INTERCEPTOR(char*, strcat, char *to, const char *from) { // NOLINT
ENSURE_ASAN_INITED();
if (flags()->replace_str) {
uptr from_length = REAL(strlen)(from);
ASAN_READ_RANGE(from, from_length + 1);
+ uptr to_length = REAL(strlen)(to);
+ ASAN_READ_RANGE(to, to_length);
+ ASAN_WRITE_RANGE(to + to_length, from_length + 1);
+ // If the copying actually happens, the |from| string should not overlap
+ // with the resulting string starting at |to|, which has a length of
+ // to_length + from_length + 1.
if (from_length > 0) {
- uptr to_length = REAL(strlen)(to);
- ASAN_READ_RANGE(to, to_length);
- ASAN_WRITE_RANGE(to + to_length, from_length + 1);
- CHECK_RANGES_OVERLAP("strcat", to, to_length + 1, from, from_length + 1);
+ CHECK_RANGES_OVERLAP("strcat", to, from_length + to_length + 1,
+ from, from_length + 1);
}
}
return REAL(strcat)(to, from); // NOLINT
@@ -425,15 +431,16 @@
INTERCEPTOR(char*, strncat, char *to, const char *from, uptr size) {
ENSURE_ASAN_INITED();
- if (flags()->replace_str && size > 0) {
+ if (flags()->replace_str) {
uptr from_length = MaybeRealStrnlen(from, size);
- ASAN_READ_RANGE(from, Min(size, from_length + 1));
+ uptr copy_length = Min(size, from_length + 1);
+ ASAN_READ_RANGE(from, copy_length);
uptr to_length = REAL(strlen)(to);
ASAN_READ_RANGE(to, to_length);
ASAN_WRITE_RANGE(to + to_length, from_length + 1);
if (from_length > 0) {
- CHECK_RANGES_OVERLAP("strncat", to, to_length + 1,
- from, Min(size, from_length + 1));
+ CHECK_RANGES_OVERLAP("strncat", to, to_length + copy_length + 1,
+ from, copy_length);
}
}
return REAL(strncat)(to, from, size);
diff --git a/lib/asan/tests/asan_test.cc b/lib/asan/tests/asan_test.cc
index 8e967e9..994e78a 100644
--- a/lib/asan/tests/asan_test.cc
+++ b/lib/asan/tests/asan_test.cc
@@ -1226,8 +1226,9 @@
strcat(to, from);
strcat(to, from);
strcat(to + from_size, from + from_size - 2);
- // Catenate empty string is not always an error.
- strcat(to - 1, from + from_size - 1);
+ // Passing an invalid pointer is an error even when concatenating an empty
+ // string.
+ EXPECT_DEATH(strcat(to - 1, from + from_size - 1), LeftOOBErrorMessage(1));
// One of arguments points to not allocated memory.
EXPECT_DEATH(strcat(to - 1, from), LeftOOBErrorMessage(1));
EXPECT_DEATH(strcat(to, from - 1), LeftOOBErrorMessage(1));
@@ -1262,8 +1263,8 @@
strncat(to, from, from_size);
from[from_size - 1] = '\0';
strncat(to, from, 2 * from_size);
- // Catenating empty string is not an error.
- strncat(to - 1, from, 0);
+ // Catenating empty string with an invalid string is still an error.
+ EXPECT_DEATH(strncat(to - 1, from, 0), LeftOOBErrorMessage(1));
strncat(to, from + from_size - 1, 10);
// One of arguments points to not allocated memory.
EXPECT_DEATH(strncat(to - 1, from, 2), LeftOOBErrorMessage(1));
@@ -1336,7 +1337,7 @@
str[10] = '\0';
str[20] = '\0';
strcat(str, str + 10);
- strcat(str, str + 11);
+ EXPECT_DEATH(strcat(str, str + 11), OverlapErrorMessage("strcat"));
str[10] = '\0';
strcat(str + 11, str);
EXPECT_DEATH(strcat(str, str + 9), OverlapErrorMessage("strcat"));
@@ -1347,7 +1348,7 @@
memset(str, 'z', size);
str[10] = '\0';
strncat(str, str + 10, 10); // from is empty
- strncat(str, str + 11, 10);
+ EXPECT_DEATH(strncat(str, str + 11, 10), OverlapErrorMessage("strncat"));
str[10] = '\0';
str[20] = '\0';
strncat(str + 5, str, 5);
