Skip to content

Commit

Permalink
thrift: avoid ASAN-detected test-only stack buffer overrun
Browse files Browse the repository at this point in the history
Summary:
* thrift/lib/cpp/test/TAsyncSSLSocketTest.cpp:
(SSLParseClientHelloOnePacket): Declare the local int array,
"fds" to be of length 2, (not 1) as required by "getfds".
SSLParseClientHelloTwoPackets): Likewise.
(SSLParseClientHelloMultiplePackets): Likewise.

Here is the ASAN report:

==23074== ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff5b5ad5e4 at pc 0x40c7ec bp 0x7fff5b5ad3a0 sp 0x7fff5b5ad398
READ of size 4 at 0x7fff5b5ad5e4 thread T0
#0 0x40c7eb in getfds(int*) /tmp/thrift/lib/cpp/test/TAsyncSSLSocketTest.h:656
#1 0x41a57f in TAsyncSSLSocketTest_SSLParseClientHelloOnePacket_Test::TestBody() /tmp/thrift/lib/cpp/test/TAsyncSSLSocketTest.cpp:723
#2 0x563ff8 in HandleSehExceptionsInMethodIfSupported<testing::Test, void> /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2078
#3 0x558a24 in testing::Test::Run() /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2151
#4 0x558b04 in testing::TestInfo::Run() /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2326
#5 0x558c6e in testing::TestCase::Run() /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2444
#6 0x55d41c in testing::internal::UnitTestImpl::RunAllTests() /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:4315
#7 0x55d6ff in testing::internal::UnitTestImpl::RunAllTests() /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:4232
#8 0x542961 in RUN_ALL_TESTS /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/include/gtest/gtest.h:2288
#3 0x7ffda054eefe in __libc_start_main ??:?
#10 0x40bdd8 in _start /home/engshare/third-party2/glibc/2.17/src/glibc-2.17/sysdeps/x86_64/start.S:123
Address 0x7fff5b5ad5e4 is located at offset 164 in frame <TestBody> of T0's stack:
This frame has 9 object(s):
[32, 33) 'majorVersion'
[96, 97) 'minorVersion'
[160, 164) 'fds'

Interestingly, this began showing up in my nightly test results
only around Dec 29th or so, even though the blamed diff is from months ago.

Test Plan:
Run these commands and confirm there are no more ASAN aborts:
fbconfig --sanitize=address thrift/lib/cpp/test:TAsyncSSLSocketTest
fbmake runtests

Reviewed By: [email protected], [email protected]

Subscribers: net-systems@, alandau, bmatheny, mshneer, sugak

FB internal diff: D1778820

Tasks: 2392412

Signature: t1:1778820:1421114269:aa0f1d9470fd19eed7c3101dc38a28dc360051ad
  • Loading branch information
meyering authored and viswanathgs committed Jan 14, 2015
1 parent 4684a9c commit fc8e554
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions thrift/lib/cpp/test/TAsyncSSLSocketTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,7 @@ TEST(TAsyncSSLSocketTest, SSLParseClientHelloOnePacket) {
TEventBase eventBase;
auto ctx = std::make_shared<SSLContext>();

int fds[1];
int fds[2];
getfds(fds);

int bufLen = 42;
Expand Down Expand Up @@ -758,7 +758,7 @@ TEST(TAsyncSSLSocketTest, SSLParseClientHelloTwoPackets) {
TEventBase eventBase;
auto ctx = std::make_shared<SSLContext>();

int fds[1];
int fds[2];
getfds(fds);

int bufLen = 42;
Expand Down Expand Up @@ -804,7 +804,7 @@ TEST(TAsyncSSLSocketTest, SSLParseClientHelloMultiplePackets) {
TEventBase eventBase;
auto ctx = std::make_shared<SSLContext>();

int fds[1];
int fds[2];
getfds(fds);

int bufLen = 42;
Expand Down

0 comments on commit fc8e554

Please sign in to comment.