ASAN found _two_ bugs in _Copy_vbool!

[CaseyCarter]

• _IsSingleBlockDest wasn't accounting for the fact that _DestEnd is past-the-end
• we sometimes don't realize we're done and read past the end of the input

Test coverage is forthcoming in my ASAN work. (I've run 1e7 iterations of the randomized test locally, FWIW.)

[StephanTLavavej]

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

[StephanTLavavej]

Thanks for finding and fixing these bugs so fast! 🕵️ 🔍 🛠️

[miscco]

I am not sure if that change has been necessary,

We only get into the first branch with _Last._Myoff >= _SourceShift so if _Last._Myoff == _SourceShift == 0 then we would only do *_VbDest = (*_VbDest & _CarryMask) | _CarryVal; and be done with it. because the second condition would ne been satisfied.

That said it is hell and I am really not sure if I need to add some more documentation of the various conditions

[CaseyCarter]

When _Last._Myoff == 0, _VbFirst is past-the-end of the input sequence when the for loop completes. The unguarded read on red line 3845 was out of bounds in this case, and triggered ASan:

=================================================================
==11404==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x11c5b1dee9e0 at pc 0x7ff7389645a3 bp 0x00b1fa95b7e0 sp 0x00b1fa95b7e0
READ of size 4 at 0x11c5b1dee9e0 thread T0
    #0 0x7ff7389645a2 in std::_Copy_vbool<class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>>(class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>) C:\STL\out\x64\out\inc\vector:3846
    #1 0x7ff7389627df in std::_Copy_unchecked<class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>>(class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>) C:\STL\out\x64\out\inc\xutility:4590
    #2 0x7ff7389749ea in std::copy<class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>>(class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>) C:\STL\out\x64\out\inc\xutility:4625
    #3 0x7ff738958bec in randomized_test_copy(class std::mersenne_twister_engine<unsigned __int64, 64, 312, 156, 31, -5403634167711393303, 29, 6148914691236517205, 17, 8202884508482404352, 37, -2270628950310912, 43, 6364136223846793005> &) C:\STL\tests\std\tests\GH_000625_vector_bool_optimization\test.cpp:1208
    #4 0x7ff73895c4f8 in main C:\STL\tests\std\tests\GH_000625_vector_bool_optimization\test.cpp:1317
    #5 0x7ff7389a4c58 in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #6 0x7ff7389a4bad in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #7 0x7ff7389a4a6d in __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:330
    #8 0x7ff7389a4ccd in mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:16
    #9 0x7ffbf04d257c  (C:\Windows\System32\KERNEL32.DLL+0x18001257c)
    #10 0x7ffbf124aa77  (C:\Windows\SYSTEM32\ntdll.dll+0x18005aa77)

0x11c5b1dee9e0 is located 0 bytes to the right of 16-byte region [0x11c5b1dee9d0,0x11c5b1dee9e0)
allocated by thread T0 here:
    #0 0x7ff7389a38d5 in operator new(unsigned __int64) D:\a\_work\1\s\src\vctools\asan\llvm\compiler-rt\lib\asan\asan_win_new_scalar_thunk.cpp:40
    #1 0x7ff73898b7d2 in std::_Default_allocate_traits::_Allocate(unsigned __int64) C:\STL\out\x64\out\inc\xmemory:90
    #2 0x7ff73895f79d in std::_Allocate<16, struct std::_Default_allocate_traits, 0>(unsigned __int64) C:\STL\out\x64\out\inc\xmemory:248
    #3 0x7ff73899a2af in std::allocator<unsigned int>::allocate(unsigned __int64) C:\STL\out\x64\out\inc\xmemory:982
    #4 0x7ff73895f925 in std::_Allocate_at_least_helper<class std::allocator<unsigned int>>(class std::allocator<unsigned int> &, unsigned __int64 &) C:\STL\out\x64\out\inc\xmemory:2211
    #5 0x7ff73898d375 in std::vector<unsigned int, class std::allocator<unsigned int>>::_Buy_raw(unsigned __int64) C:\STL\out\x64\out\inc\vector:1997
    #6 0x7ff73898c9bc in std::vector<unsigned int, class std::allocator<unsigned int>>::_Buy_nonzero(unsigned __int64) C:\STL\out\x64\out\inc\vector:2018
    #7 0x7ff7389608e2 in std::vector<unsigned int, class std::allocator<unsigned int>>::_Construct_n<unsigned int const &>(unsigned __int64, unsigned int const &) C:\STL\out\x64\out\inc\vector:2074
    #8 0x7ff738981099 in std::vector<unsigned int, class std::allocator<unsigned int>>::vector<unsigned int, class std::allocator<unsigned int>>(unsigned __int64, unsigned int const &, class std::allocator<unsigned int> const &) C:\STL\out\x64\out\inc\vector:623
    #9 0x7ff73897fe8d in std::_Vb_val<class std::allocator<bool>>::_Vb_val<class std::allocator<bool>>(unsigned __int64, bool const &, class std::allocator<bool> const &) C:\STL\out\x64\out\inc\vector:2801
    #10 0x7ff738981880 in std::vector<bool, class std::allocator<bool>>::vector<bool, class std::allocator<bool>>(unsigned __int64, class std::allocator<bool> const &) C:\STL\out\x64\out\inc\vector:2894
    #11 0x7ff738958482 in randomized_test_copy(class std::mersenne_twister_engine<unsigned __int64, 64, 312, 156, 31, -5403634167711393303, 29, 6148914691236517205, 17, 8202884508482404352, 37, -2270628950310912, 43, 6364136223846793005> &) C:\STL\tests\std\tests\GH_000625_vector_bool_optimization\test.cpp:1197
    #12 0x7ff73895c4f8 in main C:\STL\tests\std\tests\GH_000625_vector_bool_optimization\test.cpp:1317
    #13 0x7ff7389a4c58 in invoke_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:78
    #14 0x7ff7389a4bad in __scrt_common_main_seh D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288
    #15 0x7ff7389a4a6d in __scrt_common_main D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:330
    #16 0x7ff7389a4ccd in mainCRTStartup D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_main.cpp:16
    #17 0x7ffbf04d257c  (C:\Windows\System32\KERNEL32.DLL+0x18001257c)
    #18 0x7ffbf124aa77  (C:\Windows\SYSTEM32\ntdll.dll+0x18005aa77)

SUMMARY: AddressSanitizer: heap-buffer-overflow C:\STL\out\x64\out\inc\vector:3846 in std::_Copy_vbool<class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>>(class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_const_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>, class std::_Vb_iterator<struct std::_Wrap_alloc<class std::allocator<unsigned int>>>)
Shadow bytes around the buggy address:
  0x03fa6813dce0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x03fa6813dcf0: fa fa fd fd fa fa fd fd fa fa fd fa fa fa fd fd
  0x03fa6813dd00: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x03fa6813dd10: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x03fa6813dd20: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
=>0x03fa6813dd30: fa fa fd fd fa fa 00 00 fa fa 00 00[fa]fa 00 00
  0x03fa6813dd40: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 00
  0x03fa6813dd50: fa fa 00 00 fa fa 00 00 fa fa fa fa fa fa fa fa
  0x03fa6813dd60: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x03fa6813dd70: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x03fa6813dd80: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==11404==ABORTING
--