server: handle non-EnvoyExceptions safely if thrown in constructor. (#4173)

htuch · web-flow · commit 497efb95b986 · 2018-08-16T12:29:25.000-04:00
This came up while addressing oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=9335 in #4171. Without this PR, the server would shutdown non-gracefully, with TLS posts still possible to deleted worker 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 #4171. Signed-off-by: Harvey Tuch <htuch@google.com>
diff --git 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
@@ -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
