CVE-2021-43824: NULL Pointer Dereference (CWE-476)
value: Envoy is an open source edge and service proxy, designed for cloud-native applications. In affected versions a crafted request crashes Envoy when a CONNECT request is sent to JWT filter configured with regex match. This provides a denial of service attack vector. The only workaround is to not use regex in the JWT filter. Users are advised to upgrade.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
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/extensions/filters/http/jwt_authn/matcher.cc, test/extensions/filters/http/jwt_authn/filter_integration_test.cc, test/extensions/filters/http/jwt_authn/test_common.h
Commit date: 2022-02-22 17:58:22 +0000
Commit message: CVE-2021-43824

jwt_atuhn: fixed the crash when a CONNECT request is sent to JWT filter
configured with regex match.

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 77c94bbe7a..a5b98afcf2 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.
+* jwt_authn: fixed the crash when a CONNECT request is sent to JWT filter configured with regex match on the Host header.
 * 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``.
 
diff --git a/source/extensions/filters/http/jwt_authn/matcher.cc b/source/extensions/filters/http/jwt_authn/matcher.cc
index 34b934c033..964caf9f52 100644
--- a/source/extensions/filters/http/jwt_authn/matcher.cc
+++ b/source/extensions/filters/http/jwt_authn/matcher.cc
@@ -115,10 +115,13 @@ public:
            envoy::config::route::v3::RouteMatch::PathSpecifierCase::kSafeRegex);
   }
 
   bool matches(const Http::RequestHeaderMap& headers) const override {
     if (BaseMatcherImpl::matchRoute(headers)) {
+      if (headers.Path() == nullptr) {
+        return false;
+      }
       const Http::HeaderString& path = headers.Path()->value();
       const absl::string_view query_string = Http::Utility::findQueryStringStart(path);
       absl::string_view path_view = path.getStringView();
       path_view.remove_suffix(query_string.length());
       if (path_matcher_->match(path_view)) {
diff --git a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc
index 98fea932bc..46f5bbe589 100644
--- a/test/extensions/filters/http/jwt_authn/filter_integration_test.cc
+++ b/test/extensions/filters/http/jwt_authn/filter_integration_test.cc
@@ -324,10 +324,37 @@ TEST_P(LocalJwksIntegrationTest, FilterStateRequirement) {
     ASSERT_TRUE(response->complete());
     EXPECT_EQ(test.expected_status, response->headers().getStatusValue());
   }
 }
 
+// Verify that JWT config with RegEx matcher can handle CONNECT requests.
+TEST_P(LocalJwksIntegrationTest, ConnectRequestWithRegExMatch) {
+  config_helper_.prependFilter(getAuthFilterConfig(ExampleConfigWithRegEx, true));
+  initialize();
+
+  codec_client_ = makeHttpConnection(lookupPort("http"));
+
+  auto encoder_decoder = codec_client_->startRequest(Http::TestRequestHeaderMapImpl{
+      {":method", "CONNECT"},
+      {":authority", "host.com:80"},
+      {"authorization", "Bearer " + std::string(GoodToken)},
+  });
+  request_encoder_ = &encoder_decoder.first;
+  auto response = std::move(encoder_decoder.second);
+
+  if (downstreamProtocol() == Http::CodecType::HTTP1) {
+    // Because CONNECT requests for HTTP/1 do not include a path, they will fail
+    // to find a route match and return a 404.
+    ASSERT_TRUE(response->waitForEndStream());
+    ASSERT_TRUE(response->complete());
+    EXPECT_EQ("404", response->headers().getStatusValue());
+  } else {
+    ASSERT_TRUE(response->waitForReset());
+    ASSERT_TRUE(codec_client_->waitForDisconnect());
+  }
+}
+
 // The test case with a fake upstream for remote Jwks server.
 class RemoteJwksIntegrationTest : public HttpProtocolIntegrationTest {
 public:
   void createUpstreams() override {
     HttpProtocolIntegrationTest::createUpstreams();
diff --git a/test/extensions/filters/http/jwt_authn/test_common.h b/test/extensions/filters/http/jwt_authn/test_common.h
index 114ee7e5ac..44a57320dc 100644
--- a/test/extensions/filters/http/jwt_authn/test_common.h
+++ b/test/extensions/filters/http/jwt_authn/test_common.h
@@ -83,10 +83,41 @@ rules:
   requires:
     provider_name: "example_provider"
 bypass_cors_preflight: true
 )";
 
+const char ExampleConfigWithRegEx[] = R"(
+providers:
+  example_provider:
+    issuer: https://example.com
+    audiences:
+    - example_service
+    - http://example_service1
+    - https://example_service2/
+    remote_jwks:
+      http_uri:
+        uri: https://pubkey_server/pubkey_path
+        cluster: pubkey_cluster
+        timeout:
+          seconds: 5
+      cache_duration:
+        seconds: 600
+    forward_payload_header: sec-istio-auth-userinfo
+rules:
+- match:
+    safe_regex:
+      google_re2: {}
+      regex: "/somethig/.*"
+  requires:
+    provider_name: "example_provider"
+- match:
+    path: "/"
+  requires:
+    provider_name: "example_provider"
+bypass_cors_preflight: true
+)";
+
 // The name of provider for above config.
 const char ProviderName[] = "example_provider";
 
 // Payload:
 // {"iss":"https://example.com","sub":"test@example.com","aud":"example_service","exp":2001001001}

Fixed code: * jwt_authn: fixed the crash when a CONNECT request is sent to JWT filter configured with regex match on the Host header.
      if (headers.Path() == nullptr) {
        return false;
      }
// Verify that JWT config with RegEx matcher can handle CONNECT requests.
TEST_P(LocalJwksIntegrationTest, ConnectRequestWithRegExMatch) {
  config_helper_.prependFilter(getAuthFilterConfig(ExampleConfigWithRegEx, true));
  initialize();

  codec_client_ = makeHttpConnection(lookupPort("http"));

  auto encoder_decoder = codec_client_->startRequest(Http::TestRequestHeaderMapImpl{
      {":method", "CONNECT"},
      {":authority", "host.com:80"},
      {"authorization", "Bearer " + std::string(GoodToken)},
  });
  request_encoder_ = &encoder_decoder.first;
  auto response = std::move(encoder_decoder.second);

  if (downstreamProtocol() == Http::CodecType::HTTP1) {
    // Because CONNECT requests for HTTP/1 do not include a path, they will fail
    // to find a route match and return a 404.
    ASSERT_TRUE(response->waitForEndStream());
    ASSERT_TRUE(response->complete());
    EXPECT_EQ("404", response->headers().getStatusValue());
  } else {
    ASSERT_TRUE(response->waitForReset());
    ASSERT_TRUE(codec_client_->waitForDisconnect());
  }
}

const char ExampleConfigWithRegEx[] = R"(
providers:
  example_provider:
    issuer: https://example.com
    audiences:
    - example_service
    - http://example_service1
    - https://example_service2/
    remote_jwks:
      http_uri:
        uri: https://pubkey_server/pubkey_path
        cluster: pubkey_cluster
        timeout:
          seconds: 5
      cache_duration:
        seconds: 600
    forward_payload_header: sec-istio-auth-userinfo
rules:
- match:
    safe_regex:
      google_re2: {}
      regex: "/somethig/.*"
  requires:
    provider_name: "example_provider"
- match:
    path: "/"
  requires:
    provider_name: "example_provider"
bypass_cors_preflight: true
)";