CVE-2022-21655 (C++, published 2022-02-22T23:15Z)
Description: value: Envoy is an open source edge and service proxy, designed for cloud-native applications. The envoy common router will segfault if an internal redirect selects a route configured with direct response or redirect actions. This will result in a denial of service. As a workaround turn off internal redirects if direct response entries are configured on the same listener.
Severity: MEDIUM (CVSS v3 base score: 7.5)
Weakness: CWE-670 \u2014 Always-Incorrect Control Flow Implementation
The code contains a control flow path that does not reflect the algorithm that the path is intended to implement, leading to incorrect behavior any time this path is navigated.
Repository: https://github.com/envoyproxy/envoy
Files changed:
docs/root/version_history/current.rst
source/common/router/router.cc
test/common/router/router_test.cc
test/integration/redirect_integration_test.cc
Commit message: CVE-2022-21655

Crash with direct_response

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


Vulnerable code:
  const auto& route_name = route->routeEntry()->routeName();
      .WillOnce(testing::InvokeWithoutArgs([] {}));
  EXPECT_CALL(callbacks_, sendLocalReply(_, _, _, _, _)).WillOnce(testing::InvokeWithoutArgs([] {
  }));

Fixed code:
* data plane: fix crash when internal redirect selects a route configured with direct response or redirect actions.
  const auto& route_name = route->directResponseEntry() ? route->directResponseEntry()->routeName()
                                                        : route->routeEntry()->routeName();
using testing::InvokeWithoutArgs;
      .WillOnce(InvokeWithoutArgs([] {}));
  EXPECT_CALL(callbacks_, sendLocalReply(_, _, _, _, _)).WillOnce(InvokeWithoutArgs([] {}));
TEST_F(RouterTest, HttpInternalRedirectMatchedToDirectResponseSucceeded) {
  NiceMock<MockDirectResponseEntry> direct_response;
  std::string route_name("route-test-name");
  EXPECT_CALL(direct_response, routeName()).WillOnce(ReturnRef(route_name));

  enableRedirects();
  sendRequest();
  EXPECT_CALL(callbacks_, clearRouteCache()).WillOnce(InvokeWithoutArgs([&]() -> void {
    // Direct message route should be matched after internal redirect
    EXPECT_CALL(*callbacks_.route_, routeEntry()).WillRepeatedly(Return(nullptr));
    EXPECT_CALL(*callbacks_.route_, directResponseEntry()).WillRepeatedly(Return(&direct_response));
  }));
  EXPECT_CALL(callbacks_, recreateStream(_)).WillOnce(Return(true));

  response_decoder_->decodeHeaders(std::move(redirect_headers_), false);
  EXPECT_EQ(1U, cm_.thread_local_cluster_.cluster_.info_->stats_store_
                    .counter("upstream_internal_redirect_succeeded_total")
                    .value());

  // In production, the HCM recreateStream would have called this.
  router_.onDestroy();
  EXPECT_EQ(1, callbacks_.streamInfo()
                   .filterState()
                   ->getDataMutable<StreamInfo::UInt32Accessor>("num_internal_redirects")
                   ->value());
}

    auto handle_by_direct_response = config_helper_.createVirtualHost("handle.direct.response");
    handle_by_direct_response.mutable_routes(0)->set_name("direct_response");
    handle_by_direct_response.mutable_routes(0)->mutable_direct_response()->set_status(204);
    handle_by_direct_response.mutable_routes(0)
        ->mutable_direct_response()
        ->mutable_body()
        ->set_inline_string(EMPTY_STRING);
    config_helper_.addVirtualHost(handle_by_direct_response);

TEST_P(RedirectIntegrationTest, InternalRedirectHandledByDirectResponse) {
  useAccessLog("%RESPONSE_FLAGS% %RESPONSE_CODE% %RESPONSE_CODE_DETAILS% %RESP(test-header)%");
  // Validate that header sanitization is only called once.
  config_helper_.addConfigModifier(
      [](envoy::extensions::filters::network::http_connection_manager::v3::HttpConnectionManager&
             hcm) { hcm.set_via("via_value"); });
  initialize();

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

  default_request_headers_.setHost("handle.internal.redirect");
  IntegrationStreamDecoderPtr response =
      codec_client_->makeHeaderOnlyRequest(default_request_headers_);

  waitForNextUpstreamRequest();

  redirect_response_.setLocation("http://handle.direct.response/");
  upstream_request_->encodeHeaders(redirect_response_, true);

  ASSERT_TRUE(response->waitForEndStream());
  ASSERT_TRUE(response->complete());
  EXPECT_EQ("204", response->headers().getStatusValue());
  EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.upstream_internal_redirect_succeeded_total")
                   ->value());
  // 302 was never returned downstream
  EXPECT_EQ(0, test_server_->counter("http.config_test.downstream_rq_3xx")->value());
  EXPECT_EQ(1, test_server_->counter("http.config_test.downstream_rq_2xx")->value());
  EXPECT_THAT(waitForAccessLog(access_log_name_, 0),
              HasSubstr("302 internal_redirect test-header-value\n"));
  // No test header
  EXPECT_THAT(waitForAccessLog(access_log_name_, 1), HasSubstr("204 direct_response -\n"));
}