new file mode 100644
@@ -0,0 +1,420 @@
+From ac0948e39eb19c3325a576e152709d4cc915a7e4 Mon Sep 17 00:00:00 2001
+From: Craig Tiller <ctiller@google.com>
+Date: Thu, 1 Aug 2024 13:02:27 -0700
+Subject: [PATCH] [v1.60] [chttp2] Fix a bug in hpack error handling (#37361)
+
+PiperOrigin-RevId: 657234128
+PiperOrigin-RevId: 658458047
+
+Craig Tiller <ctiller@google.com>
+
+CVE: CVE-2024-7246
+
+Upstream-Status: Backport
+[https://github.com/grpc/grpc/pull/37361/files]
+
+Signed-off-by: Libo Chen <libo.chen.cn@windriver.com>
+---
+ .../chttp2/transport/hpack_parser.cc | 63 +++++++------
+ .../transport/chttp2/transport/hpack_parser.h | 2 +
+ .../transport/chttp2/hpack_parser_test.cc | 89 ++++++++++++++++---
+ .../transport/chttp2/hpack_sync_fuzzer.cc | 62 +++++++++++++
+ .../transport/chttp2/hpack_sync_fuzzer.proto | 3 +
+ 5 files changed, 179 insertions(+), 40 deletions(-)
+
+diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
+index 31bf464..f2fe80c 100644
+--- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc
++++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
+@@ -91,12 +91,14 @@ constexpr Base64InverseTable kBase64InverseTable;
+ class HPackParser::Input {
+ public:
+ Input(grpc_slice_refcount* current_slice_refcount, const uint8_t* begin,
+- const uint8_t* end, absl::BitGenRef bitsrc, HpackParseResult& error)
++ const uint8_t* end, absl::BitGenRef bitsrc,
++ HpackParseResult& frame_error, HpackParseResult& field_error)
+ : current_slice_refcount_(current_slice_refcount),
+ begin_(begin),
+ end_(end),
+ frontier_(begin),
+- error_(error),
++ frame_error_(frame_error),
++ field_error_(field_error),
+ bitsrc_(bitsrc) {}
+
+ // If input is backed by a slice, retrieve its refcount. If not, return
+@@ -215,14 +217,18 @@ class HPackParser::Input {
+
+ // Check if we saw an EOF
+ bool eof_error() const {
+- return min_progress_size_ != 0 || error_.connection_error();
++ return min_progress_size_ != 0 || frame_error_.connection_error();
++ }
++
++ // Reset the field error to be ok
++ void ClearFieldError() {
++ if (field_error_.ok()) return;
++ field_error_ = HpackParseResult();
+ }
+
+ // Minimum number of bytes to unstuck the current parse
+ size_t min_progress_size() const { return min_progress_size_; }
+
+- bool has_error() const { return !error_.ok(); }
+-
+ // Set the current error - tweaks the error to include a stream id so that
+ // chttp2 does not close the connection.
+ // Intended for errors that are specific to a stream and recoverable.
+@@ -246,10 +252,7 @@ class HPackParser::Input {
+ // read prior to being able to get further in this parse.
+ void UnexpectedEOF(size_t min_progress_size) {
+ GPR_ASSERT(min_progress_size > 0);
+- if (min_progress_size_ != 0 || error_.connection_error()) {
+- GPR_DEBUG_ASSERT(eof_error());
+- return;
+- }
++ if (eof_error()) return;
+ // Set min progress size, taking into account bytes parsed already but not
+ // consumed.
+ min_progress_size_ = min_progress_size + (begin_ - frontier_);
+@@ -302,13 +305,18 @@ class HPackParser::Input {
+ // Do not use this directly, instead use SetErrorAndContinueParsing or
+ // SetErrorAndStopParsing.
+ void SetError(HpackParseResult error) {
+- if (!error_.ok() || min_progress_size_ > 0) {
+- if (error.connection_error() && !error_.connection_error()) {
+- error_ = std::move(error); // connection errors dominate
++ SetErrorFor(frame_error_, error);
++ SetErrorFor(field_error_, std::move(error));
++ }
++
++ void SetErrorFor(HpackParseResult& error, HpackParseResult new_error) {
++ if (!error.ok() || min_progress_size_ > 0) {
++ if (new_error.connection_error() && !error.connection_error()) {
++ error = std::move(new_error); // connection errors dominate
+ }
+ return;
+ }
+- error_ = std::move(error);
++ error = std::move(new_error);
+ }
+
+ // Refcount if we are backed by a slice
+@@ -320,7 +328,8 @@ class HPackParser::Input {
+ // Frontier denotes the first byte past successfully processed input
+ const uint8_t* frontier_;
+ // Current error
+- HpackParseResult& error_;
++ HpackParseResult& frame_error_;
++ HpackParseResult& field_error_;
+ // If the error was EOF, we flag it here by noting how many more bytes would
+ // be needed to make progress
+ size_t min_progress_size_ = 0;
+@@ -597,6 +606,7 @@ class HPackParser::Parser {
+ bool ParseTop() {
+ GPR_DEBUG_ASSERT(state_.parse_state == ParseState::kTop);
+ auto cur = *input_->Next();
++ input_->ClearFieldError();
+ switch (cur >> 4) {
+ // Literal header not indexed - First byte format: 0000xxxx
+ // Literal header never indexed - First byte format: 0001xxxx
+@@ -702,7 +712,7 @@ class HPackParser::Parser {
+ break;
+ }
+ gpr_log(
+- GPR_DEBUG, "HTTP:%d:%s:%s: %s%s", log_info_.stream_id, type,
++ GPR_INFO, "HTTP:%d:%s:%s: %s%s", log_info_.stream_id, type,
+ log_info_.is_client ? "CLI" : "SVR", memento.md.DebugString().c_str(),
+ memento.parse_status == nullptr
+ ? ""
+@@ -951,11 +961,10 @@ class HPackParser::Parser {
+ state_.string_length)
+ : String::Parse(input_, state_.is_string_huff_compressed,
+ state_.string_length);
+- HpackParseResult& status = state_.frame_error;
+ absl::string_view key_string;
+ if (auto* s = absl::get_if<Slice>(&state_.key)) {
+ key_string = s->as_string_view();
+- if (status.ok()) {
++ if (state_.field_error.ok()) {
+ auto r = ValidateKey(key_string);
+ if (r != ValidateMetadataResult::kOk) {
+ input_->SetErrorAndContinueParsing(
+@@ -965,7 +974,7 @@ class HPackParser::Parser {
+ } else {
+ const auto* memento = absl::get<const HPackTable::Memento*>(state_.key);
+ key_string = memento->md.key();
+- if (status.ok() && memento->parse_status != nullptr) {
++ if (state_.field_error.ok() && memento->parse_status != nullptr) {
+ input_->SetErrorAndContinueParsing(*memento->parse_status);
+ }
+ }
+@@ -992,16 +1001,16 @@ class HPackParser::Parser {
+ key_string.size() + value.wire_size + hpack_constants::kEntryOverhead;
+ auto md = grpc_metadata_batch::Parse(
+ key_string, std::move(value_slice), state_.add_to_table, transport_size,
+- [key_string, &status, this](absl::string_view message, const Slice&) {
+- if (!status.ok()) return;
++ [key_string, this](absl::string_view message, const Slice&) {
++ if (!state_.field_error.ok()) return;
+ input_->SetErrorAndContinueParsing(
+ HpackParseResult::MetadataParseError(key_string));
+ gpr_log(GPR_ERROR, "Error parsing '%s' metadata: %s",
+ std::string(key_string).c_str(),
+ std::string(message).c_str());
+ });
+- HPackTable::Memento memento{std::move(md),
+- status.PersistentStreamErrorOrNullptr()};
++ HPackTable::Memento memento{
++ std::move(md), state_.field_error.PersistentStreamErrorOrNullptr()};
+ input_->UpdateFrontier();
+ state_.parse_state = ParseState::kTop;
+ if (state_.add_to_table) {
+@@ -1163,13 +1172,13 @@ grpc_error_handle HPackParser::Parse(
+ std::vector<uint8_t> buffer = std::move(unparsed_bytes_);
+ return ParseInput(
+ Input(nullptr, buffer.data(), buffer.data() + buffer.size(), bitsrc,
+- state_.frame_error),
++ state_.frame_error, state_.field_error),
+ is_last, call_tracer);
+ }
+- return ParseInput(
+- Input(slice.refcount, GRPC_SLICE_START_PTR(slice),
+- GRPC_SLICE_END_PTR(slice), bitsrc, state_.frame_error),
+- is_last, call_tracer);
++ return ParseInput(Input(slice.refcount, GRPC_SLICE_START_PTR(slice),
++ GRPC_SLICE_END_PTR(slice), bitsrc, state_.frame_error,
++ state_.field_error),
++ is_last, call_tracer);
+ }
+
+ grpc_error_handle HPackParser::ParseInput(
+diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.h b/src/core/ext/transport/chttp2/transport/hpack_parser.h
+index 3745668..55842e4 100644
+--- a/src/core/ext/transport/chttp2/transport/hpack_parser.h
++++ b/src/core/ext/transport/chttp2/transport/hpack_parser.h
+@@ -236,6 +236,8 @@ class HPackParser {
+ HPackTable hpack_table;
+ // Error so far for this frame (set by class Input)
+ HpackParseResult frame_error;
++ // Error so far for this field (set by class Input)
++ HpackParseResult field_error;
+ // Length of frame so far.
+ uint32_t frame_length = 0;
+ // Length of the string being parsed
+diff --git a/test/core/transport/chttp2/hpack_parser_test.cc b/test/core/transport/chttp2/hpack_parser_test.cc
+index 3772d90..d5b9c6c 100644
+--- a/test/core/transport/chttp2/hpack_parser_test.cc
++++ b/test/core/transport/chttp2/hpack_parser_test.cc
+@@ -440,19 +440,82 @@ INSTANTIATE_TEST_SUITE_P(
+ Test{"Base64LegalEncoding",
+ {},
+ {},
+- {// Binary metadata: created using:
+- // tools/codegen/core/gen_header_frame.py
+- // --compression inc --no_framing --output hexstr
+- // < test/core/transport/chttp2/bad-base64.headers
+- {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
+- "27732074756573646179",
+- absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+- "illegal base64 encoding"),
+- 0},
+- {"be",
+- absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
+- "illegal base64 encoding"),
+- 0}}},
++ {
++ // Binary metadata: created using:
++ // tools/codegen/core/gen_header_frame.py
++ // --compression inc --no_framing --output hexstr
++ // < test/core/transport/chttp2/bad-base64.headers
++ {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
++ "27732074756573646179",
++ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
++ "illegal base64 encoding"),
++ 0},
++ {"be",
++ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
++ "illegal base64 encoding"),
++ kEndOfHeaders},
++ {"82", ":method: GET\n", 0},
++ }},
++ Test{"Base64LegalEncodingWorksAfterFailure",
++ {},
++ {},
++ {
++ // Binary metadata: created using:
++ // tools/codegen/core/gen_header_frame.py
++ // --compression inc --no_framing --output hexstr
++ // < test/core/transport/chttp2/bad-base64.headers
++ {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
++ "27732074756573646179",
++ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
++ "illegal base64 encoding"),
++ 0},
++ {"be",
++ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
++ "illegal base64 encoding"),
++ 0},
++ {"400e636f6e74656e742d6c656e6774680135",
++ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
++ "illegal base64 encoding"),
++ kEndOfHeaders},
++ {"be", "content-length: 5\n", 0},
++ }},
++ Test{"Base64LegalEncodingWorksAfterFailure2",
++ {},
++ {},
++ {
++ {// Generated with: tools/codegen/core/gen_header_frame.py
++ // --compression inc --output hexstr --no_framing <
++ // test/core/transport/chttp2/MiXeD-CaSe.headers
++ "400a4d695865442d436153651073686f756c64206e6f74207061727365",
++ absl::InternalError("Illegal header key: MiXeD-CaSe"), 0},
++ // Binary metadata: created using:
++ // tools/codegen/core/gen_header_frame.py
++ // --compression inc --no_framing --output hexstr
++ // < test/core/transport/chttp2/bad-base64.headers
++ {"4009612e622e632d62696e1c6c75636b696c7920666f722075732c206974"
++ "27732074756573646179",
++ absl::InternalError("Illegal header key: MiXeD-CaSe"), 0},
++ {"be", absl::InternalError("Illegal header key: MiXeD-CaSe"),
++ 0},
++ {"400e636f6e74656e742d6c656e6774680135",
++ absl::InternalError("Illegal header key: MiXeD-CaSe"),
++ kEndOfHeaders},
++ {"be", "content-length: 5\n", 0},
++ {"bf",
++ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
++ "illegal base64 encoding"),
++ 0},
++ // Only the first error in each frame is reported, so we should
++ // still see the same error here...
++ {"c0",
++ absl::InternalError("Error parsing 'a.b.c-bin' metadata: "
++ "illegal base64 encoding"),
++ kEndOfHeaders},
++ // ... but if we look at the next frame we should see the
++ // stored error
++ {"c0", absl::InternalError("Illegal header key: MiXeD-CaSe"),
++ kEndOfHeaders},
++ }},
+ Test{"TeIsTrailers",
+ {},
+ {},
+diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.cc b/test/core/transport/chttp2/hpack_sync_fuzzer.cc
+index 47e4265..9afa41f 100644
+--- a/test/core/transport/chttp2/hpack_sync_fuzzer.cc
++++ b/test/core/transport/chttp2/hpack_sync_fuzzer.cc
+@@ -85,6 +85,10 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
+ // Not an interesting case to fuzz
+ continue;
+ }
++ if (msg.check_ab_preservation() &&
++ header.literal_inc_idx().key() == "a") {
++ continue;
++ }
+ if (absl::EndsWith(header.literal_inc_idx().value(), "-bin")) {
+ std::ignore = encoder.EmitLitHdrWithBinaryStringKeyIncIdx(
+ Slice::FromCopiedString(header.literal_inc_idx().key()),
+@@ -96,6 +100,10 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
+ }
+ break;
+ case hpack_sync_fuzzer::Header::kLiteralNotIdx:
++ if (msg.check_ab_preservation() &&
++ header.literal_not_idx().key() == "a") {
++ continue;
++ }
+ if (absl::EndsWith(header.literal_not_idx().value(), "-bin")) {
+ encoder.EmitLitHdrWithBinaryStringKeyNotIdx(
+ Slice::FromCopiedString(header.literal_not_idx().key()),
+@@ -114,6 +122,10 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
+ break;
+ }
+ }
++ if (msg.check_ab_preservation()) {
++ std::ignore = encoder.EmitLitHdrWithNonBinaryStringKeyIncIdx(
++ Slice::FromCopiedString("a"), Slice::FromCopiedString("b"));
++ }
+
+ // STAGE 2: Decode the buffer (encode_output) into a list of headers
+ HPackParser parser;
+@@ -140,6 +152,21 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
+ }
+ }
+
++ if (seen_errors.empty() && msg.check_ab_preservation()) {
++ std::string backing;
++ auto a_value = read_metadata.GetStringValue("a", &backing);
++ if (!a_value.has_value()) {
++ fprintf(stderr, "Expected 'a' header to be present: %s\n",
++ read_metadata.DebugString().c_str());
++ abort();
++ }
++ if (a_value != "b") {
++ fprintf(stderr, "Expected 'a' header to be 'b', got '%s'\n",
++ std::string(*a_value).c_str());
++ abort();
++ }
++ }
++
+ // STAGE 3: If we reached here we either had a stream error or no error
+ // parsing.
+ // Either way, the hpack tables should be of the same size between client and
+@@ -168,6 +195,41 @@ void FuzzOneInput(const hpack_sync_fuzzer::Msg& msg) {
+ }
+ abort();
+ }
++
++ if (msg.check_ab_preservation()) {
++ SliceBuffer encode_output_2;
++ hpack_encoder_detail::Encoder encoder_2(
++ &compressor, msg.use_true_binary_metadata(), encode_output_2);
++ encoder_2.EmitIndexed(62);
++ GPR_ASSERT(encode_output_2.Count() == 1);
++ grpc_metadata_batch read_metadata_2(arena.get());
++ parser.BeginFrame(
++ &read_metadata_2, 1024, 1024, HPackParser::Boundary::EndOfHeaders,
++ HPackParser::Priority::None,
++ HPackParser::LogInfo{3, HPackParser::LogInfo::kHeaders, false});
++ auto err = parser.Parse(encode_output_2.c_slice_at(0), true,
++ absl::BitGenRef(proto_bit_src),
++ /*call_tracer=*/nullptr);
++ if (!err.ok()) {
++ fprintf(stderr, "Error parsing preservation encoded data: %s\n",
++ err.ToString().c_str());
++ abort();
++ }
++ std::string backing;
++ auto a_value = read_metadata_2.GetStringValue("a", &backing);
++ if (!a_value.has_value()) {
++ fprintf(stderr,
++ "Expected 'a' header to be present: %s\nfirst metadata: %s\n",
++ read_metadata_2.DebugString().c_str(),
++ read_metadata.DebugString().c_str());
++ abort();
++ }
++ if (a_value != "b") {
++ fprintf(stderr, "Expected 'a' header to be 'b', got '%s'\n",
++ std::string(*a_value).c_str());
++ abort();
++ }
++ }
+ }
+
+ } // namespace
+diff --git a/test/core/transport/chttp2/hpack_sync_fuzzer.proto b/test/core/transport/chttp2/hpack_sync_fuzzer.proto
+index 72792b6..2c075a6 100644
+--- a/test/core/transport/chttp2/hpack_sync_fuzzer.proto
++++ b/test/core/transport/chttp2/hpack_sync_fuzzer.proto
+@@ -44,4 +44,7 @@ message Msg {
+ repeated Header headers = 2;
+ grpc.testing.FuzzConfigVars config_vars = 3;
+ repeated uint64 random_numbers = 4;
++ // Ensure that a header "a: b" appended to headers with hpack incremental
++ // indexing is correctly added to the hpack table.
++ bool check_ab_preservation = 5;
+ }
+--
+2.44.1
+
@@ -24,6 +24,7 @@ SRCREV_grpc = "e5ae3b6b44bf3b64d24bfb4b4f82556239b986db"
BRANCH = "v1.60.x"
SRC_URI = "gitsm://github.com/grpc/grpc.git;protocol=https;name=grpc;branch=${BRANCH} \
file://0001-cmake-Link-with-libatomic-on-rv32-rv64.patch \
+ file://CVE-2024-7246.patch \
"
# Fixes build with older compilers 4.8 especially on ubuntu 14.04
CXXFLAGS:append:class-native = " -Wl,--no-as-needed"