[cfe-dev] annotating libc++ to catch buffer overflows in vector/string with asan

[Kostya Serebryany]

Hi Marshall, Howard,

[continuing our discussion with Marshall on llvm dev meeting]
Consider the following test case:

#include <vector>
#include <assert.h>
typedef long T;
int main() {
  std::vector<T> v;
  v.push_back(0);
  v.push_back(1);
  v.push_back(2);
  assert(v.capacity() >= 4);
  T *p = &v[0];
  return p[3];  // OOPS.
}

Here we have a vector with size==3 and capacity==4, and we are reading
the element with index 3 using raw pointer arithmetic.
Today we have no way to detect this bug. AddressSanitizer is silent because
we still access the heap-allocated buffer within bounds.
I propose to annotate the libc++ sources so that AddressSanitizer can catch
these bugs.
Here is a prototype of the change.
I've only annotated __push_back_slow_path which is enough to handle the
test case above.
More methods will need to be annotated for full functionality.
Similar annotations are possible for std::string.
__sanitizer_annotate_contiguous_container is declared in
compiler-rt/include/sanitizer/common_interface_defs.h

--- include/vector      (revision 195011)
+++ include/vector      (working copy)
@@ -282,6 +282,15 @@
 #   define _LIBCPP_ASSERT(x, m) ((void)0)
 #endif

+#if __has_feature(address_sanitizer)
+extern "C" void __sanitizer_annotate_contiguous_container(void *, void *,
+                                                          void *, void *);
+#   define _LIBCPP_ANNOTATE_CONTIGUOUS_CONTAINER(beg, end, old_mid,
new_mid) \
+  __sanitizer_annotate_contiguous_container(beg, end, old_mid, new_mid)
+#else
+#   define _LIBCPP_ANNOTATE_CONTIGUOUS_CONTAINER(beg, end, old_mid,
new_mid)
+#endif
+
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
@@ -1525,7 +1534,12 @@
     // __v.push_back(_VSTD::forward<_Up>(__x));
     __alloc_traits::construct(__a, _VSTD::__to_raw_pointer(__v.__end_),
_VSTD::forward<_Up>(__x));
     __v.__end_++;
+    if (data())
+      _LIBCPP_ANNOTATE_CONTIGUOUS_CONTAINER(
+          data(), data() + capacity(), data() + size(), data() +
capacity());
     __swap_out_circular_buffer(__v);
+    _LIBCPP_ANNOTATE_CONTIGUOUS_CONTAINER(data(), data() + capacity(),
+                                          data() + capacity(), data() +
size());
 }

Running:
% clang++ -g  -O -fsanitize=address  -I$HOME/llvm/projects/libcxx/include
vect_oob.cc -fno-exceptions;  ./a.out
=================================================================
==20838==ERROR: AddressSanitizer: contiguous-container-buffer-overflow on
address 0x60300000eff8 at pc 0x465455 bp 0x7fffc0e9a9f0 sp 0x7fffc0e9a9e8
READ of size 8 at 0x60300000eff8 thread T0
    #0 0x465454 in main /tmp/vect_oob.cc:11

0x60300000eff8 is located 24 bytes inside of 32-byte region
[0x60300000efe0,0x60300000f000)
allocated by thread T0 here:
    #0 0x44f8fe in operator new(unsigned long)
    #1 0x46559d in std::__1::allocator<long>::allocate(unsigned long, void
const*)
    #2 0x46559d in std::__1::allocator_traits<std::__1::allocator<long>
>::allocate(std::__1::allocator<long>&, unsigned long)
    #3 0x46559d in std::__1::vector<long, std::__1::allocator<long>
>::size() const
    #4 0x46559d in void std::__1::vector<long, std::__1::allocator<long>
>::__push_back_slow_path<long const>(long const&)


Do you like the idea?
Any comments about the prototype patch?
Any suggestion how to test these annotations in libc++ (we need some kind
of DEATH tests)?

Thanks,

--kcc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131118/db478074/attachment.html>