[compiler-rt] r284901 - [sanitizers] support strict_string_checks for strncmp

[Nick Lewycky via llvm-commits]

On 21 October 2016 at 16:52, Kostya Serebryany via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: kcc
> Date: Fri Oct 21 18:52:26 2016
> New Revision: 284901
>
> URL: http://llvm.org/viewvc/llvm-project?rev=284901&view=rev
> Log:
> [sanitizers] support strict_string_checks for strncmp
>
> Added:
>     compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c
> Modified:
>     compiler-rt/trunk/lib/sanitizer_common/sanitizer_
> common_interceptors.inc
>
> Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_
> common_interceptors.inc
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/
> sanitizer_common/sanitizer_common_interceptors.inc?rev=
> 284901&r1=284900&r2=284901&view=diff
> ============================================================
> ==================
> --- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
> (original)
> +++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
> Fri Oct 21 18:52:26 2016
> @@ -304,8 +304,8 @@ INTERCEPTOR(int, strncmp, const char *s1
>      c2 = (unsigned char)s2[i];
>      if (c1 != c2 || c1 == '\0') break;
>    }
> -  COMMON_INTERCEPTOR_READ_RANGE(ctx, s1, Min(i + 1, size));
> -  COMMON_INTERCEPTOR_READ_RANGE(ctx, s2, Min(i + 1, size));
> +  COMMON_INTERCEPTOR_READ_STRING(ctx, s1, Min(i + 1, size));
> +  COMMON_INTERCEPTOR_READ_STRING(ctx, s2, Min(i + 1, size));
>

I think this change is wrong, but please double check my analysis. I think
arguments to strncmp don't need to be NUL terminated like arguments to
strcmp do.

C99 7.1.1 defines: "A string is a contiguous sequence of characters
terminated by and including the first null character." Then strcmp is
defined as a function which "compares the strings" in 7.21.4.2 while
strncmp is defined to operate over two "possibly null-terminated array"s in
7.21.4.4. The bug manifests in COMMON_INTERCEPTOR_READ_STRING calling
REAL(strlen) on these arguments which aren't guaranteed to be NUL
terminated.

What do you think?

Nick

   int result = CharCmpX(c1, c2);
>    CALL_WEAK_INTERCEPTOR_HOOK(__sanitizer_weak_hook_strncmp,
> GET_CALLER_PC(), s1,
>                               s2, size, result);
>
> Added: compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c
> URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/
> test/asan/TestCases/strncmp_strict.c?rev=284901&view=auto
> ============================================================
> ==================
> --- compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c (added)
> +++ compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c Fri Oct 21
> 18:52:26 2016
> @@ -0,0 +1,26 @@
> +// Test strict_string_checks option in strncmp function
> +// RUN: %clang_asan %s -o %t && %run %t 2>&1
> +// RUN: %env_asan_opts=strict_string_checks=false %run %t 2>&1
> +// RUN: %env_asan_opts=strict_string_checks=true not %run %t 2>&1 |
> FileCheck %s
> +
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +int main(int argc, char **argv) {
> +  size_t size = 100;
> +  char fill = 'o';
> +  char *s1 = (char*)malloc(size);
> +  memset(s1, fill, size);
> +  char *s2 = (char*)malloc(size);
> +  memset(s2, fill, size);
> +  s1[size - 1] = 'z';
> +  s2[size - 1] = 'x';
> +  int r = strncmp(s1, s2, size + 1);
> +  // CHECK: {{.*ERROR: AddressSanitizer: heap-buffer-overflow on address}}
> +  // CHECK: READ of size 101
> +  assert(r == 1);
> +  free(s1);
> +  free(s2);
> +  return 0;
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161108/bdeb5db0/attachment.html>