Custom wrappers for DFSanitizing sprintf & snprintf

[martignlo]

Summary

Introduce custom wrappers for sprintf & snprintf to propagate the labels of the arguments to the output string.

[kcc]

Note that internal_vsnprintf does not implement the full spec of vsnprintf

[pcc]

Note that internal_vsnprintf does not implement the full spec of vsnprintf

Looks like a naming conflict -- the code does end up calling the libc vsnprintf. I've asked for the name to be changed to avoid this confusion.

lib/dfsan/dfsan_custom.cc

878

It is confusing to give this function the same name as an existing function in sanitizer_common.

909

Likewise, this should probably have a different name.

1101

This can just be va_arg(ap, dfsan_label *) as pointer types are not affected by default argument promotion.

test/dfsan/custom.c

777

Please add a test for each occurrence of the PRINTF macro in the implementation.

[martignlo]

lib/dfsan/dfsan_custom.cc

878

I realize the name was not optimal but I did not realize the collision. Renamed.

909

Done

1101

Done

test/dfsan/custom.c

777

Done

[pcc]

lib/dfsan/dfsan_custom.cc

916

Can you use InternalMmapVector here?

test/dfsan/custom.c

805

Please modify this macro to check labels (in both the unlabelled and labelled cases). You already have a test for %s above so you don't need to check it here.

Though I would much prefer if this could be done without macros. Maybe we can move these tests to a .cc file and use templates?

[martignlo]

lib/dfsan/dfsan_custom.cc

916

Done.

test/dfsan/custom.c

805

I renamed the file to custom.cc so that we can use a template. Now the tests also labels the arguments and makes sure the labels are applied to the correct part of the output buffer.

Do you think it is necessary to try the same passing a zero-label?

[martignlo]

test/dfsan/custom.c

805

Ok. Now each string is formatted twice, with and without a labelled argument.

[pcc]

lib/dfsan/dfsan_custom.cc

911

InternalMmapVector<Chunk> should be simpler?

[martignlo]

lib/dfsan/dfsan_custom.cc

911

Done

[pcc]

LGTM

[martignlo]

Closed by commit rL219293 (authored by @martignlo).

lib/dfsan/dfsan_custom.cc

 //===----------------------------------------------------------------------===//
 
 #include "sanitizer_common/sanitizer_internal_defs.h"
 #include "sanitizer_common/sanitizer_linux.h"
 
 #include "dfsan/dfsan.h"
 
 #include <arpa/inet.h>
+#include <assert.h>
 #include <ctype.h>
 #include <dlfcn.h>
 #include <link.h>
 #include <poll.h>
 #include <pthread.h>
 #include <pwd.h>
 #include <sched.h>
 #include <signal.h>
+#include <stdarg.h>
+#include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <sys/resource.h>
 #include <sys/select.h>
 #include <sys/stat.h>
 #include <sys/time.h>
 #include <sys/types.h>
write_callback_info.write_callback_trampoline(
         write_callback_info.write_callback,
         fd, buf, count,
         fd_label, buf_label, count_label);
   }
 
   *ret_label = 0;
   return write(fd, buf, count);
 }
+
+// Type used to extract a dfsan_label with va_arg()
+typedef int dfsan_label_va;
+
+// A chunk of data representing the output of formatting either a constant
+// string or a single format directive.
+struct Chunk {
+  // Address of the beginning of the formatted string
+  const char *ptr;
+  // Size of the formatted string
+  size_t size;
+
+  // Type of DFSan label (depends on the format directive)
+  enum {
+    // Constant string, no argument and thus no label
+    NONE = 0,
+    // Label for an argument of '%n'
+    IGNORED,
+    // Label for a '%s' argument
+    STRING,
+    // Label for any other type of argument
+    NUMERIC,
+  } label_type;
+
+  // Value of the argument (if label_type == STRING)
+  const char *arg;
+};
+
+// Formats the input. The output is stored in 'str' starting from offset
+// 'off'. The format directive is represented by the first 'format_size' bytes
+// of 'format'. If 'has_size' is true, 'size' bounds the number of output
+// bytes. Returns the return value of the vsnprintf call used to format the
+// input.
+static int internal_snprintf(char *str, size_t off, bool has_size, size_t size,
+                             const char *format, size_t format_size, ...) {
+  char *chunk_format = (char *) malloc(format_size + 1);
+  assert(chunk_format);
+  internal_memcpy(chunk_format, format, format_size);
+  chunk_format[format_size] = '\0';
+
+  va_list ap;
+  va_start(ap, format_size);
+  int r = 0;
+  if (has_size) {
+    r = vsnprintf(str + off, off < size ? size - off : 0, chunk_format, ap);
+  } else {
+    r = vsprintf(str + off, chunk_format, ap);
+  }
+  va_end(ap);
+
+  free(chunk_format);
+  return r;
+}
+
+// Formats the input and propagates the input labels to the output. The output
+// is stored in 'str'. If 'has_size' is true, 'size' bounds the number of
+// output bytes. 'format' and 'ap' are the format string and the list of
+// arguments for formatting. Returns the return value vsnprintf would return.
+//
+// The function tokenizes the format string in chunks representing either a
+// constant string or a single format directive (e.g., '%.3f') and formats each
+// chunk independently into the output string. This approach allows to figure
+// out which bytes of the output string depends on which argument and thus to
+// propagate labels more precisely.
+static int internal_vsnprintf(char *str, bool has_size, size_t size,
+                              const char *format, va_list ap) {
+  Chunk *chunks = nullptr;
+  size_t num_chunks = 0;
+  size_t off = 0;
+
+  while (*format) {
+    chunks = (Chunk *)realloc(chunks, sizeof(Chunk) * ++num_chunks);
+    assert(chunks);
+    Chunk *chunk = chunks + (num_chunks - 1);
+    chunk->ptr = str + off;
+    chunk->arg = nullptr;
+
+    int status = 0;
+
+    if (*format != '%') {
+      // Ordinary character. Consume all the characters until a '%' or the end
+      // of the string.
+      size_t format_size = 0;
+      for (; *format && *format != '%'; ++format, ++format_size) {}
+      status = internal_snprintf(str, off, has_size, size, format - format_size,
+                                format_size);
+      chunk->label_type = Chunk::NONE;
+    } else {
+      // Conversion directive. Consume all the characters until a conversion
+      // specifier or the end of the string.
+      bool end_format = false;
+#define SPRINTF(t)                                                  \
+  internal_snprintf(str, off, has_size, size, format - format_size, \
+                    format_size + 1, va_arg(ap, t))
+
+      for (size_t format_size = 1; *++format && !end_format; ++format_size) {
+        switch (*format) {
+          case 'd':
+          case 'i':
+          case 'o':
+          case 'u':
+          case 'x':
+          case 'X':
+            switch (*(format - 1)) {
+              case 'h':
+                // Also covers the 'hh' case (since the size of the arg is still
+                // an int).
+                status = SPRINTF(int);
+                break;
+              case 'l':
+                if (format_size >= 2 && *(format - 2) == 'l') {
+                  status = SPRINTF(long long int);
+                } else {
+                  status = SPRINTF(long int);
+                }
+                break;
+              case 'q':
+                status = SPRINTF(long long int);
+                break;
+              case 'j':
+                status = SPRINTF(intmax_t);
+                break;
+              case 'z':
+                status = SPRINTF(size_t);
+                break;
+              case 't':
+                status = SPRINTF(size_t);
+                break;
+              default:
+                status = SPRINTF(int);
+            }
+            chunk->label_type = Chunk::NUMERIC;
+            end_format = true;
+            break;
+
+          case 'a':
+          case 'A':
+          case 'e':
+          case 'E':
+          case 'f':
+          case 'F':
+          case 'g':
+          case 'G':
+            if (*(format - 1) == 'L') {
+              status = SPRINTF(long double);
+            } else {
+              status = SPRINTF(double);
+            }
+            chunk->label_type = Chunk::NUMERIC;
+            end_format = true;
+            break;
+
+          case 'c':
+            status = SPRINTF(int);
+            chunk->label_type = Chunk::NUMERIC;
+            end_format = true;
+            break;
+
+          case 's':
+            chunk->arg = va_arg(ap, char *);
+            status =
+                internal_snprintf(str, off, has_size, size,
+                                  format - format_size, format_size + 1,
+                                  chunk->arg);
+            chunk->label_type = Chunk::STRING;
+            end_format = true;
+            break;
+
+          case 'p':
+            status = SPRINTF(void *);
+            chunk->label_type = Chunk::NUMERIC;
+            end_format = true;
+            break;
+
+          case 'n':
+            *(va_arg(ap, int *)) = (int)off;
+            chunk->label_type = Chunk::IGNORED;
+            end_format = true;
+            break;
+
+          case '%':
+            status = internal_snprintf(str, off, has_size, size,
+                                       format - format_size, format_size + 1);
+            chunk->label_type = Chunk::NONE;
+            end_format = true;
+            break;
+
+          default:
+            break;
+        }
+      }
+#undef SPRINTF
+    }
+
+    if (status < 0) {
+      free(chunks);
+      return status;
+    }
+
+    // A return value of {v,}snprintf of size or more means that the output was
+    // truncated.
+    if (has_size) {
+      if (off < size) {
+        size_t ustatus = (size_t) status;
+        chunk->size = ustatus >= (size - off) ?
+            ustatus - (size - off) : ustatus;
+      } else {
+        chunk->size = 0;
+      }
+    } else {
+      chunk->size = status;
+    }
+    off += status;
+  }
+
+  // Consume the labels of the output buffer, (optional) size, and format
+  // string.
+  //
+  // TODO(martignlo): Decide how to combine labels (e.g., whether to ignore or
+  // not the label of the format string).
+  va_arg(ap, dfsan_label_va);
+  if (has_size) {
+    va_arg(ap, dfsan_label_va);
+  }
+  va_arg(ap, dfsan_label_va);
+
+  // Label each output chunk according to the label supplied as argument to the
+  // function. We need to go through all the chunks and arguments even if the
+  // string was only partially printed ({v,}snprintf case).
+  for (size_t i = 0; i < num_chunks; ++i) {
+    const Chunk *chunk = chunks + i;
+
+    switch (chunk->label_type) {
+      case Chunk::NONE:
+        dfsan_set_label(0, (void*) chunk->ptr, chunk->size);
+        break;
+      case Chunk::IGNORED:
+        va_arg(ap, dfsan_label_va);
+        dfsan_set_label(0, (void*) chunk->ptr, chunk->size);
+        break;
+      case Chunk::NUMERIC: {
+        dfsan_label label = va_arg(ap, dfsan_label_va);
+        dfsan_set_label(label, (void*) chunk->ptr, chunk->size);
+        break;
+      }
+      case Chunk::STRING: {
+        // Consume the label of the pointer to the string
+        va_arg(ap, dfsan_label_va);
+        internal_memcpy(shadow_for((void *) chunk->ptr),
+                        shadow_for((void *) chunk->arg),
+                        sizeof(dfsan_label) * (strlen(chunk->arg)));
+        break;
+      }
+    }
+  }
+
+  dfsan_label *ret_label_ptr = (dfsan_label *) va_arg(ap, dfsan_label_va *);
+  *ret_label_ptr = 0;
+
+  if (chunks) {
+    free(chunks);
+  }
+
+  // Number of bytes written in total.
+  return off;
+}
+
+SANITIZER_INTERFACE_ATTRIBUTE
+int __dfsw_sprintf(char *str, const char *format, ...) {
+  va_list ap;
+  va_start(ap, format);
+  int ret = internal_vsnprintf(str, false, 0, format, ap);
+  va_end(ap);
+  return ret;
+}
+
+SANITIZER_INTERFACE_ATTRIBUTE
+int __dfsw_snprintf(char *str, size_t size, const char *format, ...) {
+  va_list ap;
+  va_start(ap, format);
+  int ret = internal_vsnprintf(str, true, size, format, ap);
+  va_end(ap);
+  return ret;
+}
 }

lib/dfsan/done_abilist.txt

 fun:getpwuid_r=custom
 fun:poll=custom
 fun:sched_getaffinity=custom
 fun:select=custom
 fun:sigemptyset=custom
 fun:sigaction=custom
 fun:gettimeofday=custom
 
+# sprintf-like
+fun:sprintf=custom
+fun:snprintf=custom
+
 # TODO: custom
-fun:snprintf=discard
-fun:vsnprintf=discard
 fun:asprintf=discard
 fun:qsort=discard
 
 ###############################################################################
 # pthread
 ###############################################################################
 fun:pthread_equal=discard
 fun:pthread_getspecific=discard

lib/sanitizer_common/scripts/check_lint.sh

 ASAN_TEST_LINT_FILTER=${COMMON_LINT_FILTER},-runtime/sizeof,-runtime/int,-runtime/printf,-runtime/threadsafe_fn
 ASAN_LIT_TEST_LINT_FILTER=${ASAN_TEST_LINT_FILTER},-whitespace/line_length
 TSAN_RTL_LINT_FILTER=${COMMON_LINT_FILTER}
 TSAN_TEST_LINT_FILTER=${TSAN_RTL_LINT_FILTER},-runtime/threadsafe_fn,-runtime/int
 TSAN_LIT_TEST_LINT_FILTER=${TSAN_TEST_LINT_FILTER},-whitespace/line_length
 MSAN_RTL_LINT_FILTER=${COMMON_LINT_FILTER}
 LSAN_RTL_LINT_FILTER=${COMMON_LINT_FILTER}
 LSAN_LIT_TEST_LINT_FILTER=${LSAN_RTL_LINT_FILTER},-whitespace/line_length
-DFSAN_RTL_LINT_FILTER=${COMMON_LINT_FILTER},-runtime/int,-runtime/printf,-runtime/references
+DFSAN_RTL_LINT_FILTER=${COMMON_LINT_FILTER},-runtime/int,-runtime/printf,-runtime/references,-readability/function
 COMMON_RTL_INC_LINT_FILTER=${COMMON_LINT_FILTER},-runtime/int,-runtime/sizeof,-runtime/printf,-readability/fn_size
 SANITIZER_INCLUDES_LINT_FILTER=${COMMON_LINT_FILTER},-runtime/int
 MKTEMP="mktemp -q /tmp/tmp.XXXXXXXXXX"
 cd ${LLVM_CHECKOUT}
 
 EXITSTATUS=0
 ERROR_LOG=$(${MKTEMP})
 

test/dfsan/custom.c

void test_dfsan_set_write_callback() {
   ASSERT_READ_ZERO_LABEL(last_buf, sizeof(last_buf));
   ASSERT_READ_ZERO_LABEL(&last_count, sizeof(last_count));
 
   // Add a label to write() arguments.  Check that the labels are readable from
   // the values passed to the callback.
   dfsan_set_label(i_label, &fd, sizeof(fd));
   dfsan_set_label(j_label, &(buf[3]), 1);
   dfsan_set_label(k_label, &buf_len, sizeof(buf_len));
-  
   res = write(fd, buf, buf_len);
   assert(write_callback_count == 2);
   ASSERT_READ_ZERO_LABEL(&res, sizeof(res));
   ASSERT_READ_LABEL(&last_fd, sizeof(last_fd), i_label);
   ASSERT_READ_LABEL(&last_buf[3], sizeof(last_buf[3]), j_label);
   ASSERT_READ_LABEL(last_buf, sizeof(last_buf), j_label);
   ASSERT_READ_LABEL(&last_count, sizeof(last_count), k_label);
 
void test_write() {
 
   // The value returned by write() should have no label.
   res = write(fd, buf, len);
   ASSERT_ZERO_LABEL(res);
 
   close(fd);
 }
 
+void test_sprintf() {
+  char buf[2048];
+  memset(buf, 'a', sizeof(buf));
+  dfsan_set_label(0, buf, sizeof(buf));
+  const char* s = "world";
+  int m = 8;
+  int d = 27;
+  dfsan_set_label(k_label, (void *) (s + 1), 2);
+  dfsan_set_label(i_label, &m, sizeof(m));
+  dfsan_set_label(j_label, &d, sizeof(d));
+  int n;
+  int r = sprintf(buf, "hello %s, %-d/%d/%d %f %n%d", s, 2014, m, d,
+                  12345.6781234, &n, 1000);
+  assert(r == 40);
+  assert(strcmp(buf, "hello world, 2014/8/27 12345.678123 1000") == 0);
+  ASSERT_READ_LABEL(buf, 7, 0);
+  ASSERT_READ_LABEL(buf + 7, 2, k_label);
+  ASSERT_READ_LABEL(buf + 9, 9, 0);
+  ASSERT_READ_LABEL(buf + 18, 1, i_label);
+  ASSERT_READ_LABEL(buf + 19, 1, 0);
+  ASSERT_READ_LABEL(buf + 20, 2, j_label);
+  ASSERT_READ_LABEL(buf + 22, 13, 0);
+  ASSERT_LABEL(r, 0);
+  assert(n == 36);
+}
+
+void test_snprintf() {
+  char buf[2048];
+  memset(buf, 'a', sizeof(buf));
+  dfsan_set_label(0, buf, sizeof(buf));
+  const char* s = "world";
+  int y = 2014;
+  int m = 8;
+  int d = 27;
+  dfsan_set_label(k_label, (void *) (s + 1), 2);
+  dfsan_set_label(i_label, &y, sizeof(y));
+  dfsan_set_label(j_label, &m, sizeof(m));
+  int r = snprintf(buf, 19, "hello %s, %-d/%d/%d %f", s, y, m, d,
+                   12345.6781234);
+  // The return value is the number of bytes that would have been written to
+  // the final string if enough space had been available.
+  assert(r == 35);
+  assert(memcmp(buf, "hello world, 2014/", 19) == 0);
+  ASSERT_READ_LABEL(buf, 7, 0);
+  ASSERT_READ_LABEL(buf + 7, 2, k_label);
+  ASSERT_READ_LABEL(buf + 9, 4, 0);
+  ASSERT_READ_LABEL(buf + 13, 4, i_label);
+  ASSERT_READ_LABEL(buf + 17, 2, 0);
+  ASSERT_LABEL(r, 0);
+}
+
 int main(void) {
   i_label = dfsan_create_label("i", 0);
   j_label = dfsan_create_label("j", 0);
   k_label = dfsan_create_label("k", 0);
   i_j_label = dfsan_union(i_label, j_label);
 
   test_calloc();
   test_clock_gettime();
int main(void) {
   test_poll();
   test_pread();
   test_pthread_create();
   test_read();
   test_sched_getaffinity();
   test_select();
   test_sigaction();
   test_sigemptyset();
+  test_snprintf();
   test_socketpair();
+  test_sprintf();
   test_stat();
   test_strcasecmp();
   test_strchr();
   test_strcmp();
   test_strcpy();
   test_strdup();
   test_strlen();
   test_strncasecmp();