[PATCH] D30384: [asan] Add an interceptor for strtok

[Manuel Rigger via Phabricator via llvm-commits]

mrigger updated this revision to Diff 89941.
mrigger added a comment.
Herald added a subscriber: kubamracek.

Thanks a lot for your feedback @ygribov! I improved the test case based on it. Additionally, I changed the interceptor to use `ASAN_READ_STRING_OF_LEN` instead of directly calling `ASAN_READ_RANGE`.  However, I think I have not yet understood why using `strict_strings` should only verify the string until the first delimiter. If I select the passed `n` to be smaller than `strlen(str) + 1` then the test case fails, since it does not detect the overflow. What did I understand wrong? Should I instead implement the test case in `projects/compiler-rt/test/asan/TestCases/` and set `RUN: %env_asan_opts=strict_string_checks=true`? If yes, should I select `n` to be `strlen(str)` (like the other interceptors) or `1`?


https://reviews.llvm.org/D30384

Files:
  lib/asan/asan_interceptors.cc
  lib/asan/tests/asan_str_test.cc


Index: lib/asan/tests/asan_str_test.cc
===================================================================
--- lib/asan/tests/asan_str_test.cc
+++ lib/asan/tests/asan_str_test.cc
@@ -107,6 +107,32 @@
   free(heap_string);
 }
 
+TEST(AddressSanitizer, StrTokTest) {
+  size_t size = Ident(100);
+  size_t token_delim_size = Ident(3);
+  char *s = MallocAndMemsetString(size, 'a');
+  char *token_delim = MallocAndMemsetString(token_delim_size, 'b');
+  s[size - 1] = '\0';
+  token_delim[token_delim_size - 1] = '\0';
+  // Normal call.
+  strtok(s, token_delim);
+  // Argument points to not-allocated memory.
+  EXPECT_DEATH(Ident(strtok(token_delim - 1, s)),
+     LeftOOBReadMessage(1));
+  EXPECT_DEATH(Ident(strtok(token_delim, s - 1)),
+     LeftOOBReadMessage(1));
+  EXPECT_DEATH(Ident(strtok(token_delim + token_delim_size, s)),
+     RightOOBReadMessage(0));
+  EXPECT_DEATH(Ident(strtok(token_delim, s + size)),
+     RightOOBReadMessage(0));
+  // Cause out-of-bounds read by missing NULL terminator.
+  s[size - 1] = 'z';
+  EXPECT_DEATH(Ident(strtok(token_delim, s)), RightOOBReadMessage(0));
+  free(s);
+  free(token_delim);
+}
+
+
 #if SANITIZER_TEST_HAS_STRNLEN
 TEST(AddressSanitizer, StrNLenOOBTest) {
   size_t size = Ident(123);
Index: lib/asan/asan_interceptors.cc
===================================================================
--- lib/asan/asan_interceptors.cc
+++ lib/asan/asan_interceptors.cc
@@ -538,6 +538,19 @@
   return REAL(strcpy)(to, from);  // NOLINT
 }
 
+INTERCEPTOR(char*, strtok, char *str, const char *delimiters) {
+  void *ctx;
+  ASAN_INTERCEPTOR_ENTER(ctx, strtok);
+  ENSURE_ASAN_INITED();
+  if (flags()->replace_str) {
+    uptr del_length = REAL(strlen)(delimiters);
+    uptr str_length = REAL(strlen)(str);
+    ASAN_READ_STRING_OF_LEN(ctx, delimiters, del_length, del_length + 1);
+    ASAN_READ_STRING_OF_LEN(ctx, str, str_length, str_length + 1);
+  }
+  return REAL(strtok)(str, delimiters);
+}
+
 INTERCEPTOR(char*, strdup, const char *s) {
   void *ctx;
   ASAN_INTERCEPTOR_ENTER(ctx, strdup);
@@ -716,6 +729,7 @@
   ASAN_INTERCEPT_FUNC(strcat);  // NOLINT
   ASAN_INTERCEPT_FUNC(strcpy);  // NOLINT
   ASAN_INTERCEPT_FUNC(wcslen);
+  ASAN_INTERCEPT_FUNC(strtok);
   ASAN_INTERCEPT_FUNC(strncat);
   ASAN_INTERCEPT_FUNC(strncpy);
   ASAN_INTERCEPT_FUNC(strdup);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D30384.89941.patch
Type: text/x-patch
Size: 2322 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170227/67f5bf51/attachment.bin>