Skip to content

Commit

Permalink
server: handle non-EnvoyExceptions safely if thrown in constructor.
Browse files Browse the repository at this point in the history
This came up while addressing oss-fuzz issue
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9335 in
envoyproxy#4171.

Without this PR, the server would shutdown non-gracefully, with TLS
posts still possible to deleted workerer thread dispatchers, resulting
in heap-user-after-free. Protobuf was throwing a CHECK exception, which
was not picked up as EnvoyException.

Risk level: Low
Testing: Unit tests added, corpus entry is in
envoyproxy#4171.

Signed-off-by: Harvey Tuch <htuch@google.com>
  • Loading branch information
htuch committed Aug 15, 2018
1 parent f06e958 commit b9053aa
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 1 deletion.
9 changes: 8 additions & 1 deletion source/server/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,14 @@ InstanceImpl::InstanceImpl(Options& options, Network::Address::InstanceConstShar
} catch (const EnvoyException& e) {
ENVOY_LOG(critical, "error initializing configuration '{}': {}", options.configPath(),
e.what());

terminate();
throw;
} catch (const std::exception& e) {
ENVOY_LOG(critical, "error initializing due to unexpected exception: {}", e.what());
terminate();
throw;
} catch (...) {
ENVOY_LOG(critical, "error initializing due to unknown exception");
terminate();
throw;
}
Expand Down
29 changes: 29 additions & 0 deletions test/server/server_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
using testing::HasSubstr;
using testing::InSequence;
using testing::Invoke;
using testing::InvokeWithoutArgs;
using testing::Property;
using testing::Ref;
using testing::SaveArg;
Expand Down Expand Up @@ -299,5 +300,33 @@ TEST_P(ServerInstanceImplTest, NoOptionsPassed) {
EnvoyException, "unable to read file: ");
}

// Validate that when std::exception is unexpectedly thrown, we exit safely.
// This is a regression test for when we used to crash.
TEST_P(ServerInstanceImplTest, StdExceptionThrowInConstructor) {
EXPECT_CALL(restart_, initialize(_, _)).WillOnce(InvokeWithoutArgs([] {
throw(std::runtime_error("foobar"));
}));
EXPECT_THROW_WITH_MESSAGE(initialize("test/server/node_bootstrap.yaml"), std::runtime_error,
"foobar");
}

// Neither EnvoyException nor std::exception derived.
class FakeException {
public:
FakeException(const std::string& what) : what_(what) {}
const std::string& what() const { return what_; }

const std::string what_;
};

// Validate that when a totally unknown exception is unexpectedly thrown, we
// exit safely. This is a regression test for when we used to crash.
TEST_P(ServerInstanceImplTest, UnknownExceptionThrowInConstructor) {
EXPECT_CALL(restart_, initialize(_, _)).WillOnce(InvokeWithoutArgs([] {
throw(FakeException("foobar"));
}));
EXPECT_THROW_WITH_MESSAGE(initialize("test/server/node_bootstrap.yaml"), FakeException, "foobar");
}

} // namespace Server
} // namespace Envoy

0 comments on commit b9053aa

Please sign in to comment.