<div dir="ltr"><br><div><br></div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 8, 2016 at 2:30 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nlewycky@google.com" target="_blank">nlewycky@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-">On 21 October 2016 at 16:52, Kostya Serebryany via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br></span><span class="gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: kcc<br>
Date: Fri Oct 21 18:52:26 2016<br>
New Revision: 284901<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=284901&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=284901&view=rev</a><br>
Log:<br>
[sanitizers] support strict_string_checks for strncmp<br>
<br>
Added:<br>
compiler-rt/trunk/test/asan/Te<wbr>stCases/strncmp_strict.c<br>
Modified:<br>
compiler-rt/trunk/lib/sanitize<wbr>r_common/sanitizer_common_<wbr>interceptors.inc<br>
<br>
Modified: compiler-rt/trunk/lib/sanitize<wbr>r_common/sanitizer_common_<wbr>interceptors.inc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc?rev=284901&r1=284900&r2=284901&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/compiler-rt/trunk/lib/sa<wbr>nitizer_common/sanitizer_commo<wbr>n_interceptors.inc?rev=284901&<wbr>r1=284900&r2=284901&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- compiler-rt/trunk/lib/sanitize<wbr>r_common/sanitizer_common_<wbr>interceptors.inc (original)<br>
+++ compiler-rt/trunk/lib/sanitize<wbr>r_common/sanitizer_common_<wbr>interceptors.inc Fri Oct 21 18:52:26 2016<br>
@@ -304,8 +304,8 @@ INTERCEPTOR(int, strncmp, const char *s1<br>
c2 = (unsigned char)s2[i];<br>
if (c1 != c2 || c1 == '\0') break;<br>
}<br>
- COMMON_INTERCEPTOR_READ_RANGE(<wbr>ctx, s1, Min(i + 1, size));<br>
- COMMON_INTERCEPTOR_READ_RANGE(<wbr>ctx, s2, Min(i + 1, size));<br>
+ COMMON_INTERCEPTOR_READ_STRING<wbr>(ctx, s1, Min(i + 1, size));<br>
+ COMMON_INTERCEPTOR_READ_STRING<wbr>(ctx, s2, Min(i + 1, size));<br></blockquote><div><br></div></span><div>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.</div><div><br></div><div>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.</div><div><br></div><div>What do you think?</div></div></div></div></blockquote><div><br></div><div>Mmmm. Are these parts equivalent in C99 and C++ standards? </div><div>At least here <a href="http://www.cplusplus.com/reference/cstring/strncmp/">http://www.cplusplus.com/reference/cstring/strncmp/</a> I read: </div><div><dt style="background-color:rgb(240,240,240);font-family:monospace;margin-top:5px;color:rgb(0,0,0);font-size:12px">str1</dt><dd style="margin-bottom:10px;color:rgb(0,0,0);font-family:verdana,arial,helvetica,sans-serif;font-size:12px">C string to be compared.</dd><dt style="background-color:rgb(240,240,240);font-family:monospace;margin-top:5px;color:rgb(0,0,0);font-size:12px">str2</dt><dd style="margin-bottom:10px;color:rgb(0,0,0);font-family:verdana,arial,helvetica,sans-serif;font-size:12px">C string to be compared.</dd></div><div><br></div><div>I probably still need to replace this with something like </div><div><div>COMMON_INTERCEPTOR_READ_RANGE((ctx), (s), \</div><div> common_flags()->strict_string_checks ? size : (Min(i + 1, size)) )</div></div><div>to avoid calling strlen</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="gmail-HOEnZb"><font color="#888888"><div><br></div><div>Nick</div></font></span><div><div class="gmail-h5"><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
int result = CharCmpX(c1, c2);<br>
CALL_WEAK_INTERCEPTOR_HOOK(__<wbr>sanitizer_weak_hook_strncmp, GET_CALLER_PC(), s1,<br>
s2, size, result);<br>
<br>
Added: compiler-rt/trunk/test/asan/Te<wbr>stCases/strncmp_strict.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/asan/TestCases/strncmp_strict.c?rev=284901&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/compiler-rt/trunk/test/<wbr>asan/TestCases/strncmp_strict.<wbr>c?rev=284901&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- compiler-rt/trunk/test/asan/Te<wbr>stCases/strncmp_strict.c (added)<br>
+++ compiler-rt/trunk/test/asan/Te<wbr>stCases/strncmp_strict.c Fri Oct 21 18:52:26 2016<br>
@@ -0,0 +1,26 @@<br>
+// Test strict_string_checks option in strncmp function<br>
+// RUN: %clang_asan %s -o %t && %run %t 2>&1<br>
+// RUN: %env_asan_opts=strict_string_c<wbr>hecks=false %run %t 2>&1<br>
+// RUN: %env_asan_opts=strict_string_c<wbr>hecks=true not %run %t 2>&1 | FileCheck %s<br>
+<br>
+#include <assert.h><br>
+#include <stdlib.h><br>
+#include <string.h><br>
+<br>
+int main(int argc, char **argv) {<br>
+ size_t size = 100;<br>
+ char fill = 'o';<br>
+ char *s1 = (char*)malloc(size);<br>
+ memset(s1, fill, size);<br>
+ char *s2 = (char*)malloc(size);<br>
+ memset(s2, fill, size);<br>
+ s1[size - 1] = 'z';<br>
+ s2[size - 1] = 'x';<br>
+ int r = strncmp(s1, s2, size + 1);<br>
+ // CHECK: {{.*ERROR: AddressSanitizer: heap-buffer-overflow on address}}<br>
+ // CHECK: READ of size 101<br>
+ assert(r == 1);<br>
+ free(s1);<br>
+ free(s2);<br>
+ return 0;<br>
+}<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>