Skip to content

Commit

Permalink
test: fix a race in ResetLargeResponseUponReceivingHeaders
Browse files Browse the repository at this point in the history 
This test wants to send a stream reset when the remote has the response EOD buffered. However, nothing prevented the request being fully completed before the reset was sent. If that happened, a UAF would happen as observed by ASAN:
```
==12094==ERROR: AddressSanitizer: heap-use-after-free on address 0x607000296ab0 at pc 0x000004b55448 bp 0x7ffe4d1e97a0 sp 0x7ffe4d1e9798
READ of size 8 at 0x607000296ab0 thread T0
    #0 0x4b55447 in Envoy::IntegrationCodecClient::sendReset(Envoy::Http::RequestEncoder&) /proc/self/cwd/test/integration/http_integration.cc:159:3
    envoyproxy#1 0x494bfda in Envoy::ProtocolIntegrationTest_ResetLargeResponseUponReceivingHeaders_Test::TestBody() /proc/self/cwd/test/integration/protocol_integration_test.cc:3317:18
    envoyproxy#2 0xd58fe24 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
    envoyproxy#3 0xd557d82 in testing::Test::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2655:5
    envoyproxy#4 0xd5598e7 in testing::TestInfo::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2832:11
    envoyproxy#5 0xd55bc84 in testing::TestSuite::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2986:28
    envoyproxy#6 0xd57e03a in testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5697:44
    envoyproxy#7 0xd593e23 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
    envoyproxy#8 0xd57cd46 in testing::UnitTest::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5280:10
    envoyproxy#9 0xa30daa4 in Envoy::TestRunner::RunTests(int, char**) /proc/self/cwd/external/com_google_googletest/googletest/include/gtest/gtest.h:2485:46
    envoyproxy#10 0xa3091f7 in main /proc/self/cwd/test/main.cc:34:10
    envoyproxy#11 0x7efe24527082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)
    envoyproxy#12 0x47300ad in _start (/mnt/ssd/cas/work/1/exec/bazel-out/k8-dbg/bin/test/integration/protocol_integration_test.runfiles/envoy/test/integration/protocol_integration_test+0x47300ad)

0x607000296ab0 is located 48 bytes inside of 80-byte region [0x607000296a80,0x607000296ad0)
freed by thread T0 here:
    #0 0x47b2512 in free /local/mnt/workspace/bcain_clang_hu-bcain-lv_22036/final/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:52:3
    envoyproxy#1 0x8810bb8 in Envoy::Http::CodecClient::ActiveRequest::~ActiveRequest() /proc/self/cwd/./source/common/http/codec_client.h:220:10
    envoyproxy#2 0x60bac89 in std::__1::unique_ptr<Envoy::Event::DeferredDeletable, std::__1::default_delete<Envoy::Event::DeferredDeletable> >::reset(Envoy::Event::DeferredDeletable*) /opt/llvm/bin/../include/c++/v1/__memory/unique_ptr.h:54:5
    envoyproxy#3 0xa548578 in Envoy::Event::DispatcherImpl::clearDeferredDeleteList() /proc/self/cwd/source/common/event/dispatcher_impl.cc:142:21
    envoyproxy#4 0xa55b56f in void std::__1::__invoke_void_return_wrapper<void, true>::__call<Envoy::Event::DispatcherImpl::DispatcherImpl(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, Envoy::Thread::ThreadFactory&, Envoy::TimeSource&, Envoy::Random::RandomGenerator&, Envoy::Filesystem::Instance&, Envoy::Event::TimeSystem&, std::__1::function<std::__1::unique_ptr<Envoy::Event::ScaledRangeTimerManager, std::__1::default_delete<Envoy::Event::ScaledRangeTimerManager> > (Envoy::Event::Dispatcher&)> const&, std::__1::shared_ptr<Envoy::Buffer::WatermarkFactory> const&)::$_2&>(Envoy::Event::DispatcherImpl::DispatcherImpl(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, Envoy::Thread::ThreadFactory&, Envoy::TimeSource&, Envoy::Random::RandomGenerator&, Envoy::Filesystem::Instance&, Envoy::Event::TimeSystem&, std::__1::function<std::__1::unique_ptr<Envoy::Event::ScaledRangeTimerManager, std::__1::default_delete<Envoy::Event::ScaledRangeTimerManager> > (Envoy::Event::Dispatcher&)> const&, std::__1::shared_ptr<Envoy::Buffer::WatermarkFactory> const&)::$_2&) /proc/self/cwd/source/common/event/dispatcher_impl.cc:79:30
    envoyproxy#5 0xa55b293 in std::__1::__function::__func<Envoy::Event::DispatcherImpl::DispatcherImpl(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, Envoy::Thread::ThreadFactory&, Envoy::TimeSource&, Envoy::Random::RandomGenerator&, Envoy::Filesystem::Instance&, Envoy::Event::TimeSystem&, std::__1::function<std::__1::unique_ptr<Envoy::Event::ScaledRangeTimerManager, std::__1::default_delete<Envoy::Event::ScaledRangeTimerManager> > (Envoy::Event::Dispatcher&)> const&, std::__1::shared_ptr<Envoy::Buffer::WatermarkFactory> const&)::$_2, std::__1::allocator<Envoy::Event::DispatcherImpl::DispatcherImpl(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, Envoy::Thread::ThreadFactory&, Envoy::TimeSource&, Envoy::Random::RandomGenerator&, Envoy::Filesystem::Instance&, Envoy::Event::TimeSystem&, std::__1::function<std::__1::unique_ptr<Envoy::Event::ScaledRangeTimerManager, std::__1::default_delete<Envoy::Event::ScaledRangeTimerManager> > (Envoy::Event::Dispatcher&)> const&, std::__1::shared_ptr<Envoy::Buffer::WatermarkFactory> const&)::$_2>, void ()>::operator()() /opt/llvm/bin/../include/c++/v1/__functional/function.h:180:16
    envoyproxy#6 0x4a5d789 in std::__1::__function::__value_func<void ()>::operator()() const /opt/llvm/bin/../include/c++/v1/__functional/function.h:507:16
    envoyproxy#7 0xab04334 in Envoy::Event::SchedulableCallbackImpl::SchedulableCallbackImpl(Envoy::CSmartPtr<event_base, &(event_base_free)>&, std::__1::function<void ()>)::$_0::__invoke(int, short, void*) /opt/llvm/bin/../include/c++/v1/__functional/function.h:1184:12
    envoyproxy#8 0xb77d84e in event_process_active_single_queue /mnt/ssd/cas/work/3/exec/external/com_github_libevent_libevent/event.c:1713:4
    envoyproxy#9 0xb75ee42 in event_process_active /mnt/ssd/cas/work/3/exec/external/com_github_libevent_libevent/event.c
    envoyproxy#10 0xb75ee42 in event_base_loop /mnt/ssd/cas/work/3/exec/external/com_github_libevent_libevent/event.c:2047:12
    envoyproxy#11 0xaaff6cc in Envoy::Event::LibeventScheduler::run(Envoy::Event::Dispatcher::RunType) /proc/self/cwd/source/common/event/libevent_scheduler.cc:60:3
    envoyproxy#12 0xa552a24 in Envoy::Event::DispatcherImpl::run(Envoy::Event::Dispatcher::RunType) /proc/self/cwd/source/common/event/dispatcher_impl.cc:299:19
    envoyproxy#13 0x4b5335d in Envoy::IntegrationCodecClient::flushWrite() /proc/self/cwd/test/integration/http_integration.cc:100:29
    envoyproxy#14 0x4b5492b in Envoy::IntegrationCodecClient::sendData(Envoy::Http::RequestEncoder&, absl::string_view, bool) /proc/self/cwd/test/integration/http_integration.cc:137:3
    envoyproxy#15 0x494be79 in Envoy::ProtocolIntegrationTest_ResetLargeResponseUponReceivingHeaders_Test::TestBody() /proc/self/cwd/test/integration/protocol_integration_test.cc:3312:18
    envoyproxy#16 0xd58fe24 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
    envoyproxy#17 0xd557d82 in testing::Test::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2655:5
    envoyproxy#18 0xd5598e7 in testing::TestInfo::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2832:11
    envoyproxy#19 0xd55bc84 in testing::TestSuite::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2986:28
    envoyproxy#20 0xd57e03a in testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5697:44
    envoyproxy#21 0xd593e23 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
    envoyproxy#22 0xd57cd46 in testing::UnitTest::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5280:10
    envoyproxy#23 0xa30daa4 in Envoy::TestRunner::RunTests(int, char**) /proc/self/cwd/external/com_google_googletest/googletest/include/gtest/gtest.h:2485:46
    envoyproxy#24 0xa3091f7 in main /proc/self/cwd/test/main.cc:34:10
    envoyproxy#25 0x7efe24527082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)

previously allocated by thread T0 here:
    #0 0x47b27be in malloc /local/mnt/workspace/bcain_clang_hu-bcain-lv_22036/final/llvm-project/compiler-rt/lib/asan/asan_malloc_linux.cpp:69:3
    envoyproxy#1 0x7efe247613d4 in operator new(unsigned long) (/opt/llvm/lib/x86_64-unknown-linux-gnu/libc++abi.so.1+0x2c3d4)
    envoyproxy#2 0x4b55ae1 in Envoy::IntegrationCodecClient::startRequest(Envoy::Http::RequestHeaderMap const&, bool) /proc/self/cwd/test/integration/http_integration.cc:176:35
    envoyproxy#3 0x494bb90 in Envoy::ProtocolIntegrationTest_ResetLargeResponseUponReceivingHeaders_Test::TestBody() /proc/self/cwd/test/integration/protocol_integration_test.cc:3303:41
    envoyproxy#4 0xd58fe24 in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
    envoyproxy#5 0xd557d82 in testing::Test::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2655:5
    envoyproxy#6 0xd5598e7 in testing::TestInfo::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2832:11
    envoyproxy#7 0xd55bc84 in testing::TestSuite::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2986:28
    envoyproxy#8 0xd57e03a in testing::internal::UnitTestImpl::RunAllTests() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5697:44
    envoyproxy#9 0xd593e23 in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:2580:10
    envoyproxy#10 0xd57cd46 in testing::UnitTest::Run() /proc/self/cwd/external/com_google_googletest/googletest/src/gtest.cc:5280:10
    envoyproxy#11 0xa30daa4 in Envoy::TestRunner::RunTests(int, char**) /proc/self/cwd/external/com_google_googletest/googletest/include/gtest/gtest.h:2485:46
    envoyproxy#12 0xa3091f7 in main /proc/self/cwd/test/main.cc:34:10
    envoyproxy#13 0x7efe24527082 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x24082) (BuildId: 1878e6b475720c7c51969e69ab2d276fae6d1dee)

```

To fix this, use flow control to prevent the fake upstream from sending all its data before a reset is sent.

Signed-off-by: Benjamin Peterson <benjamin@engflow.com>
  • Loading branch information
@benjaminp
benjaminp committed Sep 13, 2022
1 parent a88f146 commit daaa4d2
Showing 1 changed file with 20 additions and 11 deletions.
31 changes: 20 additions & 11 deletions test/integration/protocol_integration_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3290,8 +3290,6 @@ TEST_P(ProtocolIntegrationTest, ResetLargeResponseUponReceivingHeaders) {
if (downstreamProtocol() == Http::CodecType::HTTP1) {
return;
}
autonomous_upstream_ = true;
autonomous_allow_incomplete_streams_ = true;
initialize();

envoy::config::core::v3::Http2ProtocolOptions http2_options =
Expand All @@ -3302,22 +3300,33 @@ TEST_P(ProtocolIntegrationTest, ResetLargeResponseUponReceivingHeaders) {
codec_client_ = makeRawHttpConnection(makeClientConnection(lookupPort("http")), http2_options);

// The response is larger than the stream flow control window. So the reset of it will
// likely be buffered QUIC stream send buffer.
// likely be buffered in the QUIC stream send buffer.
constexpr uint64_t response_size = 100 * 1024;
auto encoder_decoder = codec_client_->startRequest(
Http::TestRequestHeaderMapImpl{{":method", "POST"},
{":path", "/"},
{":scheme", "http"},
{":authority", "sni.lyft.com"},
{"content-length", "10"},
{"response_size_bytes", absl::StrCat(response_size)}});

auto encoder_decoder =
codec_client_->startRequest(Http::TestRequestHeaderMapImpl{{":method", "POST"},
{":path", "/"},
{":scheme", "http"},
{":authority", "sni.lyft.com"},
{"content-length", "10"}});
auto& encoder = encoder_decoder.first;

std::string data(10, 'a');
codec_client_->sendData(encoder, data, true);

ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));
upstream_request_->encodeHeaders(
Http::TestResponseHeaderMapImpl{{":status", "200"},
{"content-length", absl::StrCat(response_size)}},
false);

auto response = std::move(encoder_decoder.second);
response->waitForHeaders();
// Reset stream while the quic server stream might have FIN buffered in its send buffer.
encoder.getStream().readDisable(true);
upstream_request_->encodeData(response_size, true);
ASSERT_TRUE(fake_upstream_connection_->waitForNoPost());
// Reset stream while the quic server stream has FIN buffered in its send buffer.
codec_client_->sendReset(encoder);
codec_client_->close();
}
Expand Down

0 comments on commit daaa4d2

Please sign in to comment.