From b9053aa3da51fa2cc6b5fdc97b9dfd6e3af8ed86 Mon Sep 17 00:00:00 2001
From: Harvey Tuch <htuch@google.com>
Date: Wed, 15 Aug 2018 08:43:54 -0400
Subject: [PATCH] server: handle non-EnvoyExceptions safely if thrown in
 constructor.

This came up while addressing oss-fuzz issue
https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9335 in
https://github.com/envoyproxy/envoy/pull/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
https://github.com/envoyproxy/envoy/pull/4171.

Signed-off-by: Harvey Tuch <htuch@google.com>
---
 source/server/server.cc    |  9 ++++++++-
 test/server/server_test.cc | 29 +++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/source/server/server.cc b/source/server/server.cc
index cdd3d605a64f..5f3f4ddd2166 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -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;
   }
diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index 56cdbac0d562..a682d3184ae6 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -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;
@@ -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