Skip to content

Commit 948bc9b

Browse files
htuchaa-stripe
authored andcommitted
router: disallow :-prefixed custom header removal. (envoyproxy#4576)
This is the remove counterpart to envoyproxy#4220. Fixes oss-fuzz issue https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=10737. Risk Level: Low Testing: Unit test and corpus entry added. Signed-off-by: Harvey Tuch <htuch@google.com> Signed-off-by: Aaltan Ahmad <aa@stripe.com>
1 parent fa5d9a2 commit 948bc9b

File tree

3 files changed

+31
-0
lines changed

3 files changed

+31
-0
lines changed

source/common/router/header_parser.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,12 @@ HeaderParserPtr HeaderParser::configure(
240240
HeaderParserPtr header_parser = configure(headers_to_add);
241241

242242
for (const auto& header : headers_to_remove) {
243+
// We reject :-prefix (e.g. :path) removal here. This is dangerous, since other aspects of
244+
// request finalization assume their existence and they are needed for well-formedness in most
245+
// cases.
246+
if (header[0] == ':') {
247+
throw EnvoyException(":-prefixed headers may not be removed");
248+
}
243249
header_parser->headers_to_remove_.emplace_back(header);
244250
}
245251

test/common/router/config_impl_test.cc

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1006,6 +1006,30 @@ name: foo
10061006
}
10071007
}
10081008

1009+
// Validate that we can't remove :-prefixed request headers.
1010+
TEST(RouteMatcherTest, TestRequestHeadersToRemoveNoPseudoHeader) {
1011+
for (const std::string& header : {":path", ":authority", ":method", ":scheme", ":status",
1012+
":protocol", ":no-chunks", ":status"}) {
1013+
const std::string yaml = fmt::format(R"EOF(
1014+
name: foo
1015+
virtual_hosts:
1016+
- name: www2
1017+
domains: ["*"]
1018+
request_headers_to_remove:
1019+
- {}
1020+
)EOF",
1021+
header);
1022+
1023+
NiceMock<Server::Configuration::MockFactoryContext> factory_context;
1024+
NiceMock<Envoy::RequestInfo::MockRequestInfo> request_info;
1025+
1026+
envoy::api::v2::RouteConfiguration route_config = parseRouteConfigurationFromV2Yaml(yaml);
1027+
1028+
EXPECT_THROW_WITH_MESSAGE(TestConfigImpl config(route_config, factory_context, true),
1029+
EnvoyException, ":-prefixed headers may not be removed");
1030+
}
1031+
}
1032+
10091033
TEST(RouteMatcherTest, Priority) {
10101034
std::string json = R"EOF(
10111035
{
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
config { virtual_hosts { name: " " domains: "*" routes { match { path: "/" } route { cluster: " " prefix_rewrite: " " } } } request_headers_to_remove: ":path" }

0 commit comments

Comments
 (0)