grpc: Fix CVE-2024-7246

Backport patches [1] to fix CVE-2024-7246.

[1] https://github.com/grpc/grpc/pull/37361/files

Signed-off-by: Libo Chen <libo.chen.cn@windriver.com>
Signed-off-by: Armin Kuster <akuster808@gmail.com>

2 files changed, 421 insertions, 0 deletions

diff --git a/meta-oe/recipes-devtools/grpc/grpc/CVE-2024-7246.patch b/meta-oe/recipes-devtools/grpc/grpc/CVE-2024-7246.patch
new file mode 100644
index 0000000000..a690b49c67
--- /dev/null
+++ b/meta-oe/recipes-devtools/grpc/grpc/CVE-2024-7246.patch
@@ -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
+
diff --git a/meta-oe/recipes-devtools/grpc/grpc_1.60.1.bb b/meta-oe/recipes-devtools/grpc/grpc_1.60.1.bb
index 63c696a623..4557fcc908 100644
--- a/meta-oe/recipes-devtools/grpc/grpc_1.60.1.bb
+++ b/meta-oe/recipes-devtools/grpc/grpc_1.60.1.bb
@@ -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"