rapidjson
rapidjson copied to clipboard
Tencent
SIMD.SkipWhitespace_SSE2 AddressSanitizer: stack-buffer-overflow
Reproduce steps:
git clone https://github.com/Tencent/rapidjson.git && cd rapidjson
git checkout fd3dc29a && git submodule update --init --force
mkdir build && cd build
cmake .. -GNinja -DCMAKE_BUILD_TYPE=Debug -DRAPIDJSON_BUILD_THIRDPARTY_GTEST=ON -DRAPIDJSON_BUILD_CXX17=ON -DRAPIDJSON_BUILD_ASAN=ON -DRAPIDJSON_BUILD_UBSAN=ON -DRAPIDJSON_ENABLE_INSTRUMENTATION_OPT=OFF -DRAPIDJSON_HAS_STDSTRING=ON -DRAPIDJSON_USE_MEMBERSMAP=ON -DRAPIDJSON_BUILD_DOC=OFF -DRAPIDJSON_BUILD_EXAMPLES=OFF -DCMAKE_INSTALL_PREFIX=$PWD/output
ninja unittest
bin/unittest --gtest_filter=SIMD.SkipWhitespace_SSE2
Error reporting:
RapidJSON v1.1.0
Note: Google Test filter = SIMD.SkipWhitespace_SSE2
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from SIMD
[ RUN ] SIMD.SkipWhitespace_SSE2
=================================================================
==10384==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffef7770480 at pc 0x56317a77f469 bp 0x7ffef776fce0 sp 0x7ffef776fcd0
READ of size 16 at 0x7ffef7770480 thread T0
#0 0x56317a77f468 in _mm_load_si128(long long __vector(2) const*) /usr/lib/gcc/x86_64-linux-gnu/9/include/emmintrin.h:697
#1 0x56317a77f468 in rapidjson_simd::SkipWhitespace_SIMD(char const*) ../include/rapidjson/reader.h:360
#2 0x56317a77ff2a in void rapidjson_simd::SkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >(rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> >&) ../include/rapidjson/reader.h:511
#3 0x56317a782160 in void TestSkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >() ../test/unittest/simdtest.cpp:65
#4 0x56317a778936 in SIMD_SkipWhitespace_SSE2_Test::TestBody() ../test/unittest/simdtest.cpp:76
#5 0x56317aae5ec3 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2418
#6 0x56317aac7dd4 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2454
#7 0x56317aa2d321 in testing::Test::Run() ../thirdparty/gtest/googletest/src/gtest.cc:2492
#8 0x56317aa3094d in testing::TestInfo::Run() ../thirdparty/gtest/googletest/src/gtest.cc:2668
#9 0x56317aa339ce in testing::TestCase::Run() ../thirdparty/gtest/googletest/src/gtest.cc:2786
#10 0x56317aa68767 in testing::internal::UnitTestImpl::RunAllTests() ../thirdparty/gtest/googletest/src/gtest.cc:5048
#11 0x56317aaec35f in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2418
#12 0x56317aacd062 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2454
#13 0x56317aa5c987 in testing::UnitTest::Run() ../thirdparty/gtest/googletest/src/gtest.cc:4664
#14 0x56317a7b7305 in RUN_ALL_TESTS() ../thirdparty/gtest/googletest/include/gtest/gtest.h:2329
#15 0x56317a7b70e1 in main ../test/unittest/unittest.cpp:44
#16 0x7f822ff7d0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2)
#17 0x56317992793d in _start (/home/shuaiz/GitEnlistments/github.com/public/rapidjson/build/bin/unittest+0x181493d)
Address 0x7ffef7770480 is located in stack of thread T0 at offset 1392 in frame
#0 0x56317a781ca7 in void TestSkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >() ../test/unittest/simdtest.cpp:53
This frame has 12 object(s):
[48, 49) '<unknown>'
[64, 65) '<unknown>'
[80, 88) 'i' (line 63)
[112, 120) '<unknown>'
[144, 152) '<unknown>'
[176, 184) '<unknown>'
[208, 216) '<unknown>'
[240, 248) '<unknown>'
[272, 288) 's' (line 62)
[304, 320) 'gtest_ar' (line 68)
[336, 352) 'gtest_ar' (line 69)
[368, 1393) 'buffer' (line 55) <== Memory access at offset 1392 partially overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /usr/lib/gcc/x86_64-linux-gnu/9/include/emmintrin.h:697 in _mm_load_si128(long long __vector(2) const*)
Shadow bytes around the buggy address:
0x10005eee6040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10005eee6050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10005eee6060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10005eee6070: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10005eee6080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10005eee6090:[01]f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3
0x10005eee60a0: f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10005eee60b0: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 f3 f3
0x10005eee60c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10005eee60d0: 00 00 00 00 00 00 f1 f1 f1 f1 00 00 f2 f2 00 00
0x10005eee60e0: 00 00 f2 f2 f2 f2 00 00 00 00 f3 f3 f3 f3 00 00
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
Shadow gap: cc
==10384==ABORTING
Environments:
/etc/os-release:
NAME="Ubuntu"
VERSION="20.04.4 LTS (Focal Fossa)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 20.04.4 LTS"
VERSION_ID="20.04"
HOME_URL="https://www.ubuntu.com/"
SUPPORT_URL="https://help.ubuntu.com/"
BUG_REPORT_URL="https://bugs.launchpad.net/ubuntu/"
PRIVACY_POLICY_URL="https://www.ubuntu.com/legal/terms-and-policies/privacy-policy"
VERSION_CODENAME=focal
UBUNTU_CODENAME=focal
gcc --version:
gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Can reproduce on master(fcb23c2dbf561ec0798529be4f66394d3e4996d8).
Tried to align the string buffer to 16 bytes but still failed.
diff --git a/test/unittest/simdtest.cpp b/test/unittest/simdtest.cpp
index 649505fa..285d17d1 100644
--- a/test/unittest/simdtest.cpp
+++ b/test/unittest/simdtest.cpp
@@ -52,7 +52,7 @@ using namespace rapidjson_simd;
template <typename StreamType>
void TestSkipWhitespace() {
for (size_t step = 1; step < 32; step++) {
- char buffer[1025];
+ alignas(16) char buffer[1025];
for (size_t i = 0; i < 1024; i++)
buffer[i] = " \t\r\n"[i % 4];
for (size_t i = 0; i < 1024; i += step)
RapidJSON v1.1.0
Note: Google Test filter = SIMD.SkipWhitespace_SSE2
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from SIMD
[ RUN ] SIMD.SkipWhitespace_SSE2
=================================================================
==11163==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdbaedabe0 at pc 0x565151579dad bp 0x7ffdbaeda440 sp 0x7ffdbaeda430
READ of size 16 at 0x7ffdbaedabe0 thread T0
#0 0x565151579dac in _mm_load_si128(long long __vector(2) const*) /usr/lib/gcc/x86_64-linux-gnu/9/include/emmintrin.h:697
#1 0x565151579dac in rapidjson_simd::SkipWhitespace_SIMD(char const*) ../include/rapidjson/reader.h:360
#2 0x56515157a86e in void rapidjson_simd::SkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >(rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> >&) ../include/rapidjson/reader.h:511
#3 0x56515157caa4 in void TestSkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >() ../test/unittest/simdtest.cpp:65
#4 0x56515157327a in SIMD_SkipWhitespace_SSE2_Test::TestBody() ../test/unittest/simdtest.cpp:76
#5 0x5651518e0807 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2418
#6 0x5651518c2718 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2454
#7 0x565151827c65 in testing::Test::Run() ../thirdparty/gtest/googletest/src/gtest.cc:2492
#8 0x56515182b291 in testing::TestInfo::Run() ../thirdparty/gtest/googletest/src/gtest.cc:2668
#9 0x56515182e312 in testing::TestCase::Run() ../thirdparty/gtest/googletest/src/gtest.cc:2786
#10 0x5651518630ab in testing::internal::UnitTestImpl::RunAllTests() ../thirdparty/gtest/googletest/src/gtest.cc:5048
#11 0x5651518e6ca3 in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2418
#12 0x5651518c79a6 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) ../thirdparty/gtest/googletest/src/gtest.cc:2454
#13 0x5651518572cb in testing::UnitTest::Run() ../thirdparty/gtest/googletest/src/gtest.cc:4664
#14 0x5651515b1c49 in RUN_ALL_TESTS() ../thirdparty/gtest/googletest/include/gtest/gtest.h:2329
#15 0x5651515b1a25 in main ../test/unittest/unittest.cpp:44
#16 0x7f54e39f00b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2)
#17 0x56515071e93d in _start (/home/shuaiz/GitEnlistments/github.com/public/rapidjson/build/bin/unittest+0x181993d)
Address 0x7ffdbaedabe0 is located in stack of thread T0 at offset 1392 in frame
#0 0x56515157c5eb in void TestSkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >() ../test/unittest/simdtest.cpp:53
This frame has 12 object(s):
[48, 49) '<unknown>'
[64, 65) '<unknown>'
[80, 88) 'i' (line 63)
[112, 120) '<unknown>'
[144, 152) '<unknown>'
[176, 184) '<unknown>'
[208, 216) '<unknown>'
[240, 248) '<unknown>'
[272, 288) 's' (line 62)
[304, 320) 'gtest_ar' (line 68)
[336, 352) 'gtest_ar' (line 69)
[368, 1393) 'buffer' (line 55) <== Memory access at offset 1392 partially overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /usr/lib/gcc/x86_64-linux-gnu/9/include/emmintrin.h:697 in _mm_load_si128(long long __vector(2) const*)
Shadow bytes around the buggy address:
0x1000375d3520: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000375d3530: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000375d3540: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000375d3550: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000375d3560: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x1000375d3570: 00 00 00 00 00 00 00 00 00 00 00 00[01]f3 f3 f3
0x1000375d3580: f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 f3 00 00
0x1000375d3590: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000375d35a0: 00 00 00 00 f1 f1 f1 f1 00 00 f3 f3 00 00 00 00
0x1000375d35b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x1000375d35c0: 00 00 f1 f1 f1 f1 00 00 f2 f2 00 00 00 00 f2 f2
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
Shadow gap: cc
==11163==ABORTING
After a closer look, I think the string buffer is not allocated large enough.
char buffer[1025];
The SSE2 loading would load the extra 1 byte together with the 16 bytes package, so I changed 1025 to 1024 + 16 and it passed now.
This issue also happened in many other places.
PerfTest::json_PerfTest::whitespace_PerfTest::types_RapidJson::temp_- several on-stack allocated strings. (search
" { \"hello\" : \"world\", \"t\" : true , \"f\" : false, \"n\": null, \"i\":123, \"pi\": 3.1416, \"a\":[1, 2, 3, 4] } ") - several
strdup()
@hcoona As part of some debugging work on PR #1915, @ylavic supplied a more formal fix for this error, which is now in a new PR #2101. I'd like to use this issue to fix all the sanitizer errors that are appearing, if possible. This is what I am seeing when running valgrind on Fedora 36, with the PR applied.
1) SIMD.SkipWhitespace_SSE42 A couple of each of these.
==11425== Conditional jump or move depends on uninitialised value(s)
==11425== at 0x9AD3FC: SkipWhitespace_SIMD (reader.h:305)
==11425== by 0x9AD3FC: SkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<> > > (reader.h:511)
==11425== by 0x9AD3FC: void TestSkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >() (simdtest.cpp:67)
==11425== by 0x9A67A8: SIMD_SkipWhitespace_SSE42_Test::TestBody() (simdtest.cpp:78)
==11425== by 0xA65FFE: HandleSehExceptionsInMethodIfSupported<testing::Test, void> (gtest.cc:2418)
and
==11425== Use of uninitialised value of size 8
==11425== at 0x9AD41D: Peek (stream.h:159)
==11425== by 0x9AD41D: void TestSkipWhitespace<rapidjson_simd::GenericStringStream<rapidjson_simd::UTF8<char> > >() (simdtest.cpp:68)
==11425== by 0x9A67A8: SIMD_SkipWhitespace_SSE42_Test::TestBody() (simdtest.cpp:78)
==11425== by 0xA65FFE: HandleSehExceptionsInMethodIfSupported<testing::Test, void> (gtest.cc:2418)
2) Uri.Parse_UTF16
Only the test where RAPIDJSON_HAS_STDSTRING is set. Occurs at uritest.cpp:195 and up to 3 other subsequent similar lines.
==11539== Invalid read of size 32
==11539== at 0x53A9E1D: __wmemcmp_avx2_movbe (memcmp-avx2-movbe.S:413)
==11539== by 0x517B2DD: UnknownInlinedFun (char_traits.h:512)
==11539== by 0x517B2DD: std::__cxx11::basic_string<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> >::compare(wchar_t const*) const (basic_string.tcc:895)
==11539== by 0x9C9CD5: operator==<wchar_t, std::char_traits<wchar_t>, std::allocator<wchar_t> > (basic_string.h:3601)
==11539== by 0x9C9CD5: Uri_Parse_UTF16_Test::TestBody() (uritest.cpp:195)
==11539== by 0xA65FFE: HandleSehExceptionsInMethodIfSupported<testing::Test, void> (gtest.cc:2418)
Fyi @miloyip
FWIW, we encountered this same issue from this snippet of code, in the Open 3D engine: https://github.com/o3de/o3de/blob/40cdf65119022498d9e61b9b021d461cf05bdc32/Code/Framework/AzToolsFramework/Tests/Prefab/Link/PrefabLinkDomTests.cpp#L17
Note that in the code, PrefabDom is just a type alias for using PrefabDom = rapidjson::Document;
Parsing that string literal ( patchesCopy.Parse(patchesValue.data()) causes the SIMD loop to eventually run off the end of the array, to be more specific, it ends up with the last few characters in the buffer as ] (end of buffer, which is less than 16 bytes, so trying to load it into a SIMD buffer causes an access violation.
I notice the tests in rapidjson were updated to always use a SIMD-sized buffer, in their test case, and yes, that fixes the test by making it so that the test avoids any scenario where the SIZE of the buffer its reading is not a multiple of 16 bytes, but that would imply that the rapidjson::Document::Parse(const char*) api has a constraint where all input strings must always be a length of a multiple of 16 bytes and cannot end in a whitespace (?)
I'll be fixing this in the Open 3D Engine by invoking Parse with a size parameter, which calls the other variant of SIMD parse that stops before it overruns.
