CVE-2021-43826: Use After Free (CWE-416)
value: Envoy is an open source edge and service proxy, designed for cloud-native applications. In affected versions of Envoy a crash occurs when configured for :ref:`upstream tunneling <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.tunneling_config>` and the downstream connection disconnects while the the upstream connection or http/2 stream is still being established. There are no workarounds for this issue. Users are advised to upgrade.
CWE description: The product reuses or references memory after it has been freed. At some point afterward, the memory may be allocated again and saved in another pointer, while the original pointer references a location somewhere within the new allocation. Any operations using the original pointer are no longer valid because the memory "belongs" to the code that operates on the new pointer.
CVSS v3 base score: 7.5
Published: 2022-02-22T23:15Z
Repository: https://github.com/envoyproxy/envoy
Language: C++
Files changed: docs/root/version_history/current.rst, source/common/tcp_proxy/tcp_proxy.cc, source/common/tcp_proxy/tcp_proxy.h, test/integration/tcp_tunneling_integration_test.cc
Commit date: 2022-02-22 17:58:22 +0000
Commit message: CVE-2021-43826

Signed-off-by: Yan Avlasov <yavlasov@google.com>

Diff: diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst
index f420dcf359..77c94bbe7a 100644
--- a/docs/root/version_history/current.rst
+++ b/docs/root/version_history/current.rst
@@ -25,10 +25,11 @@ Bug Fixes
 *Changes expected to improve the state of the world and are unlikely to have negative effects*
 
 * access_log: fix memory leak when reopening an access log fails. Access logs will now try to be reopened on each subsequent flush attempt after a failure.
 * data plane: fixing error handling where writing to a socket failed while under the stack of processing. This should genreally affect HTTP/3. This behavioral change can be reverted by setting ``envoy.reloadable_features.allow_upstream_inline_write`` to false.
 * eds: fix the eds cluster update by allowing update on the locality of the cluster endpoints. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.support_locality_update_on_eds_cluster_endpoints`` to false.
+* tcp_proxy: fix a crash that occurs when configured for :ref:`upstream tunneling <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.tunneling_config>` and the downstream connection disconnects while the the upstream connection or http/2 stream is still being established.
 * tls: fix a bug while matching a certificate SAN with an exact value in ``match_typed_subject_alt_names`` of a listener where wildcard ``*`` character is not the only character of the dns label. Example, ``baz*.example.net`` and ``*baz.example.net`` and ``b*z.example.net`` will match ``baz1.example.net`` and ``foobaz.example.net`` and ``buzz.example.net``, respectively.
 * upstream: fix stack overflow when a cluster with large number of idle connections is removed.
 * xray: fix the AWS X-Ray tracer extension to not sample the trace if ``sampled=`` keyword is not present in the header ``x-amzn-trace-id``.
 
 Removed Config or Runtime
diff --git a/source/common/tcp_proxy/tcp_proxy.cc b/source/common/tcp_proxy/tcp_proxy.cc
index 42ef1844be..ce7a0dd45b 100644
--- a/source/common/tcp_proxy/tcp_proxy.cc
+++ b/source/common/tcp_proxy/tcp_proxy.cc
@@ -518,12 +518,18 @@ Network::FilterStatus Filter::onNewConnection() {
   }
   return initializeUpstreamConnection();
 }
 
 void Filter::onDownstreamEvent(Network::ConnectionEvent event) {
+  if (event == Network::ConnectionEvent::LocalClose ||
+      event == Network::ConnectionEvent::RemoteClose) {
+    downstream_closed_ = true;
+  }
+
   ENVOY_CONN_LOG(trace, "on downstream event {}, has upstream = {}", read_callbacks_->connection(),
                  static_cast<int>(event), upstream_ == nullptr);
+
   if (upstream_) {
     Tcp::ConnectionPool::ConnectionDataPtr conn_data(upstream_->onDownstreamEvent(event));
     if (conn_data != nullptr &&
         conn_data->connection().state() != Network::Connection::State::Closed) {
       config_->drainManager().add(config_->sharedConfig(), std::move(conn_data),
@@ -568,11 +574,13 @@ void Filter::onUpstreamEvent(Network::ConnectionEvent event) {
         getStreamInfo().setResponseFlag(StreamInfo::ResponseFlag::UpstreamConnectionFailure);
         read_callbacks_->upstreamHost()->outlierDetector().putResult(
             Upstream::Outlier::Result::LocalOriginConnectFailed);
       }
 
-      initializeUpstreamConnection();
+      if (!downstream_closed_) {
+        initializeUpstreamConnection();
+      }
     } else {
       if (read_callbacks_->connection().state() == Network::Connection::State::Open) {
         read_callbacks_->connection().close(Network::ConnectionCloseType::FlushWrite);
       }
     }
diff --git a/source/common/tcp_proxy/tcp_proxy.h b/source/common/tcp_proxy/tcp_proxy.h
index 361bbd7065..1d6935aa05 100644
--- a/source/common/tcp_proxy/tcp_proxy.h
+++ b/source/common/tcp_proxy/tcp_proxy.h
@@ -399,10 +399,11 @@ protected:
   Router::MetadataMatchCriteriaConstPtr metadata_match_criteria_;
   Network::TransportSocketOptionsConstSharedPtr transport_socket_options_;
   Network::Socket::OptionsSharedPtr upstream_options_;
   uint32_t connect_attempts_{};
   bool connecting_{};
+  bool downstream_closed_{};
 };
 
 // This class deals with an upstream connection that needs to finish flushing, when the downstream
 // connection has been closed. The TcpProxy is destroyed when the downstream connection is closed,
 // so handling the upstream connection here allows it to finish draining or timeout.
diff --git a/test/integration/tcp_tunneling_integration_test.cc b/test/integration/tcp_tunneling_integration_test.cc
index c20055b16a..ad8be22f32 100644
--- a/test/integration/tcp_tunneling_integration_test.cc
+++ b/test/integration/tcp_tunneling_integration_test.cc
@@ -814,10 +814,73 @@ TEST_P(TcpTunnelingIntegrationTest, ResetStreamTest) {
   // Reset the stream.
   upstream_request_->encodeResetStream();
   tcp_client_->waitForDisconnect();
 }
 
+TEST_P(TcpTunnelingIntegrationTest, UpstreamConnectingDownstreamDisconnect) {
+  if (upstreamProtocol() == Http::CodecType::HTTP1) {
+    return;
+  }
+
+#if defined(WIN32)
+  // TODO(ggreenway): figure out why this test fails on Windows and remove this disable.
+  // Failing tests:
+  // IpAndHttpVersions/TcpTunnelingIntegrationTest.UpstreamConnectingDownstreamDisconnect/IPv4_HttpDownstream_Http3UpstreamBareHttp2,
+  // IpAndHttpVersions/TcpTunnelingIntegrationTest.UpstreamConnectingDownstreamDisconnect/IPv6_HttpDownstream_Http2UpstreamWrappedHttp2,
+  // Times out at the end of the test on `ASSERT_TRUE(upstream_request_->waitForReset());`.
+  return;
+#endif
+
+  config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
+    envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy proxy_config;
+    proxy_config.set_stat_prefix("tcp_stats");
+    proxy_config.set_cluster("cluster_0");
+    proxy_config.mutable_tunneling_config()->set_hostname("host.com:80");
+
+    // Enable retries. The crash is due to retrying after the downstream connection is closed, which
+    // can't occur if retries are not enabled.
+    proxy_config.mutable_max_connect_attempts()->set_value(2);
+
+    auto* listeners = bootstrap.mutable_static_resources()->mutable_listeners();
+    for (auto& listener : *listeners) {
+      if (listener.name() != "tcp_proxy") {
+        continue;
+      }
+      auto* filter_chain = listener.mutable_filter_chains(0);
+      auto* filter = filter_chain->mutable_filters(0);
+      filter->mutable_typed_config()->PackFrom(proxy_config);
+
+      // Use TLS because it will respond to a TCP half-close during handshake by closing the
+      // connection.
+      envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context;
+      ConfigHelper::initializeTls({}, *tls_context.mutable_common_tls_context());
+      filter_chain->mutable_transport_socket()->set_name("envoy.transport_sockets.tls");
+      filter_chain->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context);
+
+      break;
+    }
+  });
+
+  enableHalfClose(false);
+  initialize();
+
+  IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy"));
+
+  // Wait for the request for a connection, but don't send a response back yet. This ensures that
+  // tcp_proxy is stuck in `connecting_`.
+  ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
+  ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));
+  ASSERT_TRUE(upstream_request_->waitForHeadersComplete());
+
+  // Close the client connection. The TLS transport socket will detect this even while
+  // `readDisable(true)` on the connection, and will raise a `RemoteClose` event.
+  tcp_client->close();
+
+  ASSERT_TRUE(upstream_request_->waitForReset());
+  ASSERT_TRUE(fake_upstream_connection_->close());
+}
+
 TEST_P(TcpTunnelingIntegrationTest, TestIdletimeoutWithLargeOutstandingData) {
   enableHalfClose(false);
   config_helper_.setBufferLimits(1024, 1024);
   config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
     auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(1);

Vulnerable code:       initializeUpstreamConnection();
Fixed code: * tcp_proxy: fix a crash that occurs when configured for :ref:`upstream tunneling <envoy_v3_api_field_extensions.filters.network.tcp_proxy.v3.TcpProxy.tunneling_config>` and the downstream connection disconnects while the the upstream connection or http/2 stream is still being established.
  if (event == Network::ConnectionEvent::LocalClose ||
      event == Network::ConnectionEvent::RemoteClose) {
    downstream_closed_ = true;
  }


      if (!downstream_closed_) {
        initializeUpstreamConnection();
      }
  bool downstream_closed_{};
TEST_P(TcpTunnelingIntegrationTest, UpstreamConnectingDownstreamDisconnect) {
  if (upstreamProtocol() == Http::CodecType::HTTP1) {
    return;
  }

#if defined(WIN32)
  // TODO(ggreenway): figure out why this test fails on Windows and remove this disable.
  // Failing tests:
  // IpAndHttpVersions/TcpTunnelingIntegrationTest.UpstreamConnectingDownstreamDisconnect/IPv4_HttpDownstream_Http3UpstreamBareHttp2,
  // IpAndHttpVersions/TcpTunnelingIntegrationTest.UpstreamConnectingDownstreamDisconnect/IPv6_HttpDownstream_Http2UpstreamWrappedHttp2,
  // Times out at the end of the test on `ASSERT_TRUE(upstream_request_->waitForReset());`.
  return;
#endif

  config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
    envoy::extensions::filters::network::tcp_proxy::v3::TcpProxy proxy_config;
    proxy_config.set_stat_prefix("tcp_stats");
    proxy_config.set_cluster("cluster_0");
    proxy_config.mutable_tunneling_config()->set_hostname("host.com:80");

    // Enable retries. The crash is due to retrying after the downstream connection is closed, which
    // can't occur if retries are not enabled.
    proxy_config.mutable_max_connect_attempts()->set_value(2);

    auto* listeners = bootstrap.mutable_static_resources()->mutable_listeners();
    for (auto& listener : *listeners) {
      if (listener.name() != "tcp_proxy") {
        continue;
      }
      auto* filter_chain = listener.mutable_filter_chains(0);
      auto* filter = filter_chain->mutable_filters(0);
      filter->mutable_typed_config()->PackFrom(proxy_config);

      // Use TLS because it will respond to a TCP half-close during handshake by closing the
      // connection.
      envoy::extensions::transport_sockets::tls::v3::DownstreamTlsContext tls_context;
      ConfigHelper::initializeTls({}, *tls_context.mutable_common_tls_context());
      filter_chain->mutable_transport_socket()->set_name("envoy.transport_sockets.tls");
      filter_chain->mutable_transport_socket()->mutable_typed_config()->PackFrom(tls_context);

      break;
    }
  });

  enableHalfClose(false);
  initialize();

  IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy"));

  // Wait for the request for a connection, but don't send a response back yet. This ensures that
  // tcp_proxy is stuck in `connecting_`.
  ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_));
  ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_));
  ASSERT_TRUE(upstream_request_->waitForHeadersComplete());

  // Close the client connection. The TLS transport socket will detect this even while
  // `readDisable(true)` on the connection, and will raise a `RemoteClose` event.
  tcp_client->close();

  ASSERT_TRUE(upstream_request_->waitForReset());
  ASSERT_TRUE(fake_upstream_connection_->close());
}