[compiler-rt] 94b0851 - [sanitizer] use uptr to store the result of internal_strlen/internal_strnlen in printf_common

[via llvm-commits]

Author: Enna1
Date: 2023-08-28T14:37:35+08:00
New Revision: 94b0851aad618397f508e56c63c6f928e4e911b9

URL: https://github.com/llvm/llvm-project/commit/94b0851aad618397f508e56c63c6f928e4e911b9
DIFF: https://github.com/llvm/llvm-project/commit/94b0851aad618397f508e56c63c6f928e4e911b9.diff

LOG: [sanitizer] use uptr to store the result of internal_strlen/internal_strnlen in printf_common

The return type of `internal_strlen()` is 'uptr', but in `printf_common()` we store the result of `internal_strlen()` into an 'int' type variable.
When the result value of `internal_strlen()` is larger than the largest possible value of 'int' type, the implicit conversion from 'uptr' to 'int' will change the result value to a negative value.

Without this change, asan reports a false positive negative-size-param in the added testcase.

Reviewed By: vitalybuka

Differential Revision: https://reviews.llvm.org/D157266

Added: 
    compiler-rt/test/sanitizer_common/TestCases/vsnprintf.cpp

Modified: 
    compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc
index 24485900644b38..24e5dc0fb22f5e 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc
@@ -547,24 +547,25 @@ static void printf_common(void *ctx, const char *format, va_list aq) {
       continue;
     } else if (size == FSS_STRLEN) {
       if (void *argp = va_arg(aq, void *)) {
+        uptr len;
         if (dir.starredPrecision) {
           // FIXME: properly support starred precision for strings.
-          size = 0;
+          len = 0;
         } else if (dir.fieldPrecision > 0) {
           // Won't read more than "precision" symbols.
-          size = internal_strnlen((const char *)argp, dir.fieldPrecision);
-          if (size < dir.fieldPrecision) size++;
+          len = internal_strnlen((const char *)argp, dir.fieldPrecision);
+          if (len < (uptr)dir.fieldPrecision)
+            len++;
         } else {
           // Whole string will be accessed.
-          size = internal_strlen((const char *)argp) + 1;
+          len = internal_strlen((const char *)argp) + 1;
         }
-        COMMON_INTERCEPTOR_READ_RANGE(ctx, argp, size);
+        COMMON_INTERCEPTOR_READ_RANGE(ctx, argp, len);
       }
     } else if (size == FSS_WCSLEN) {
       if (void *argp = va_arg(aq, void *)) {
         // FIXME: Properly support wide-character strings (via wcsrtombs).
-        size = 0;
-        COMMON_INTERCEPTOR_READ_RANGE(ctx, argp, size);
+        COMMON_INTERCEPTOR_READ_RANGE(ctx, argp, 0);
       }
     } else {
       // Skip non-pointer args

diff  --git a/compiler-rt/test/sanitizer_common/TestCases/vsnprintf.cpp b/compiler-rt/test/sanitizer_common/TestCases/vsnprintf.cpp
new file mode 100644
index 00000000000000..4eb9b24770f5d3
--- /dev/null
+++ b/compiler-rt/test/sanitizer_common/TestCases/vsnprintf.cpp
@@ -0,0 +1,34 @@
+// Test that the common part implementation of *printf interceptors does not
+// cause negative-size-param false positives.
+
+// RUN: %clangxx -O2 %s -o %t
+// RUN: %env_tool_opts=check_printf=1 %run %t 2>&1
+
+// FIXME: The maximum supported allocation size is too platform-specific:
+// REQUIRES: x86_64-target-arch
+
+// FIXME: printf is not intercepted on Windows yet.
+// UNSUPPORTED: target={{.*windows-msvc.*}}
+
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void write(char *buf, int buf_size, const char *fmt, ...) {
+  va_list args;
+  va_start(args, fmt);
+  vsnprintf(buf, buf_size, fmt, args);
+  va_end(args);
+}
+
+int main() {
+  char buffer[100];
+  const size_t kStrSize = 1UL << 31;
+  char *x = (char *)malloc(kStrSize);
+  memset(x, '=', kStrSize - 1);
+  x[kStrSize - 1] = 0;
+  write(buffer, 100, "%s\n", x);
+  free(x);
+  return 0;
+}