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();
