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

[Marshall Clow]

On Nov 18, 2013, at 7:25 AM, Kostya Serebryany <kcc at google.com> wrote:

> 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? 

Yes - very much so.

> Any comments about the prototype patch? 

Yes. 

1) I would implement it with inline functions other than macros.

Like this:


extern "C" void __sanitizer_annotate_contiguous_container(void *, void *, void *, void *);
void inline __annotate_contiguous container
	( const void *__beg, const void *__end, const void *__old_mid, const void *new_mid )
#if __has_feature(address_sanitizer)
	{  __sanitizer_annotate_contiguous_container(beg, end, old_mid, new_mid); }
#else
	{}
#endif

2) I'd like more information about what __sanitizer_annotate_contiguous_container does ;-)

> Any suggestion how to test these annotations in libc++ (we need some kind of DEATH tests)? 

We already have failing tests in libc++.
they are named XXXX.fail.cpp.

Getting them to fail only under ASAN will be trickier.

-- Marshall

Marshall Clow     Idio Software   <mailto:mclow.lists at gmail.com>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
        -- Yu Suzuki