Re-adds some minimal validation to NoopHeaderValidator. The minimal set of checks necessary for Envoy fuzz tests to succeed are: * header names and values may not contain \0, \r or \n * requests and responses must contain the minimal required set of pseudo-headers PiperOrigin-RevId: 587014516
diff --git a/quiche/http2/adapter/noop_header_validator.cc b/quiche/http2/adapter/noop_header_validator.cc index f39342d..b687df5 100644 --- a/quiche/http2/adapter/noop_header_validator.cc +++ b/quiche/http2/adapter/noop_header_validator.cc
@@ -1,20 +1,56 @@ #include "quiche/http2/adapter/noop_header_validator.h" #include "absl/strings/escaping.h" +#include "absl/strings/string_view.h" +#include "quiche/http2/adapter/header_validator_base.h" #include "quiche/common/platform/api/quiche_logging.h" namespace http2 { namespace adapter { +namespace { + +constexpr absl::string_view kInvalidChars("\0\r\n", 3); + +} // namespace + +void NoopHeaderValidator::StartHeaderBlock() { + HeaderValidatorBase::StartHeaderBlock(); + has_method_ = false; + has_scheme_ = false; + has_path_ = false; +} + HeaderValidatorBase::HeaderStatus NoopHeaderValidator::ValidateSingleHeader( absl::string_view key, absl::string_view value) { + if (key.empty()) { + return HEADER_FIELD_INVALID; + } + if (key.find_first_of(kInvalidChars) != absl::string_view::npos || + value.find_first_of(kInvalidChars) != absl::string_view::npos) { + return HEADER_FIELD_INVALID; + } + if (key[0] != ':') { + return HEADER_OK; + } if (key == ":status") { status_ = std::string(value); + } else if (key == ":method") { + has_method_ = true; + } else if (key == ":scheme") { + has_scheme_ = true; + } else if (key == ":path") { + has_path_ = true; } return HEADER_OK; } -bool NoopHeaderValidator::FinishHeaderBlock(HeaderType /* type */) { +bool NoopHeaderValidator::FinishHeaderBlock(HeaderType type) { + if (type == HeaderType::REQUEST) { + return has_method_ && has_scheme_ && has_path_; + } else if (type == HeaderType::RESPONSE || type == HeaderType::RESPONSE_100) { + return !status_.empty(); + } return true; }
diff --git a/quiche/http2/adapter/noop_header_validator.h b/quiche/http2/adapter/noop_header_validator.h index f6b95e9..ef78027 100644 --- a/quiche/http2/adapter/noop_header_validator.h +++ b/quiche/http2/adapter/noop_header_validator.h
@@ -13,10 +13,16 @@ public: NoopHeaderValidator() = default; + void StartHeaderBlock() override; HeaderStatus ValidateSingleHeader(absl::string_view key, absl::string_view value) override; bool FinishHeaderBlock(HeaderType type) override; + + private: + bool has_method_ = false; + bool has_scheme_ = false; + bool has_path_ = false; }; } // namespace adapter
diff --git a/quiche/http2/adapter/noop_header_validator_test.cc b/quiche/http2/adapter/noop_header_validator_test.cc index 65cc763..a6dc041 100644 --- a/quiche/http2/adapter/noop_header_validator_test.cc +++ b/quiche/http2/adapter/noop_header_validator_test.cc
@@ -5,6 +5,7 @@ #include <vector> #include "absl/strings/str_cat.h" +#include "quiche/http2/adapter/header_validator_base.h" #include "quiche/common/platform/api/quiche_test.h" namespace http2 { @@ -19,11 +20,20 @@ {":path", "/foo"}, {":scheme", "https"}}; +TEST(NoopHeaderValidatorTest, EmptyHeaderBlock) { + NoopHeaderValidator v; + v.StartHeaderBlock(); + EXPECT_FALSE(v.FinishHeaderBlock(HeaderType::REQUEST)); + + v.StartHeaderBlock(); + EXPECT_FALSE(v.FinishHeaderBlock(HeaderType::RESPONSE)); +} + TEST(NoopHeaderValidatorTest, HeaderNameEmpty) { NoopHeaderValidator v; NoopHeaderValidator::HeaderStatus status = v.ValidateSingleHeader("", "value"); - EXPECT_EQ(NoopHeaderValidator::HEADER_OK, status); + EXPECT_EQ(NoopHeaderValidator::HEADER_FIELD_INVALID, status); } TEST(NoopHeaderValidatorTest, HeaderValueEmpty) { @@ -44,27 +54,29 @@ EXPECT_EQ(NoopHeaderValidator::HEADER_OK, status); } -TEST(NoopHeaderValidatorTest, AnyNameCharIsValid) { +TEST(NoopHeaderValidatorTest, FewInvalidNameChars) { NoopHeaderValidator v; char pseudo_name[] = ":met hod"; char name[] = "na me"; for (int i = std::numeric_limits<char>::min(); i < std::numeric_limits<char>::max(); ++i) { char c = static_cast<char>(i); + const HeaderValidatorBase::HeaderStatus expected_status = + (c == '\0' || c == '\r' || c == '\n') + ? HeaderValidatorBase::HEADER_FIELD_INVALID + : HeaderValidatorBase::HEADER_OK; // Test a pseudo-header name with this char. pseudo_name[3] = c; auto sv = absl::string_view(pseudo_name, 8); - EXPECT_EQ(NoopHeaderValidator::HEADER_OK, - v.ValidateSingleHeader(sv, "value")); + EXPECT_EQ(expected_status, v.ValidateSingleHeader(sv, "value")); // Test a regular header name with this char. name[2] = c; sv = absl::string_view(name, 5); - EXPECT_EQ(NoopHeaderValidator::HEADER_OK, - v.ValidateSingleHeader(sv, "value")); + EXPECT_EQ(expected_status, v.ValidateSingleHeader(sv, "value")); } } -TEST(NoopHeaderValidatorTest, AnyValueCharIsValid) { +TEST(NoopHeaderValidatorTest, FewInvalidValueChars) { NoopHeaderValidator v; char value[] = "val ue"; for (int i = std::numeric_limits<char>::min(); @@ -72,8 +84,11 @@ char c = static_cast<char>(i); value[3] = c; auto sv = absl::string_view(value, 6); - EXPECT_EQ(NoopHeaderValidator::HEADER_OK, - v.ValidateSingleHeader("name", sv)); + const HeaderValidatorBase::HeaderStatus expected_status = + (c == '\0' || c == '\r' || c == '\n') + ? HeaderValidatorBase::HEADER_FIELD_INVALID + : HeaderValidatorBase::HEADER_OK; + EXPECT_EQ(expected_status, v.ValidateSingleHeader("name", sv)); } } @@ -103,18 +118,21 @@ } } -TEST(NoopHeaderValidatorTest, AnyAuthorityCharIsValid) { +TEST(NoopHeaderValidatorTest, FewInvalidAuthorityChars) { char value[] = "ho st.example.com"; for (int i = std::numeric_limits<char>::min(); i < std::numeric_limits<char>::max(); ++i) { char c = static_cast<char>(i); value[2] = c; auto sv = absl::string_view(value, 17); + const HeaderValidatorBase::HeaderStatus expected_status = + (c == '\0' || c == '\r' || c == '\n') + ? HeaderValidatorBase::HEADER_FIELD_INVALID + : HeaderValidatorBase::HEADER_OK; for (absl::string_view key : {":authority", "host"}) { NoopHeaderValidator v; v.StartHeaderBlock(); - EXPECT_EQ(NoopHeaderValidator::HEADER_OK, - v.ValidateSingleHeader(key, sv)); + EXPECT_EQ(expected_status, v.ValidateSingleHeader(key, sv)); } } } @@ -152,8 +170,13 @@ v.ValidateSingleHeader(to_add.first, to_add.second)); } } - // Even if a pseudo-header is missing, final validation will succeed. - EXPECT_TRUE(v.FinishHeaderBlock(HeaderType::REQUEST)); + // If the missing pseudo-header is :authority, final validation will + // succeed. Otherwise, it will fail. + if (to_skip.first == ":authority") { + EXPECT_TRUE(v.FinishHeaderBlock(HeaderType::REQUEST)); + } else { + EXPECT_FALSE(v.FinishHeaderBlock(HeaderType::REQUEST)); + } } // When all pseudo-headers are present, final validation will succeed. @@ -297,11 +320,11 @@ NoopHeaderValidator v; for (HeaderType type : {HeaderType::RESPONSE, HeaderType::RESPONSE_100}) { - // When `:status` is missing, validation succeeds. + // When `:status` is missing, validation fails. v.StartHeaderBlock(); EXPECT_EQ(NoopHeaderValidator::HEADER_OK, v.ValidateSingleHeader("foo", "bar")); - EXPECT_TRUE(v.FinishHeaderBlock(type)); + EXPECT_FALSE(v.FinishHeaderBlock(type)); // When all pseudo-headers are present, final validation succeeds. v.StartHeaderBlock();