python3-grpcio: Fix CVE-2024-7246

Apply the nearest upstream fix from v1.62.3 [1] for HPACK parser error
handling to prevent header table desynchronization, aligned with the original
fix in v1.60.2 [2] as referenced in [3].

[1] https://github.com/grpc/grpc/commit/1d172cfca56440889ca32ae516b8c2767321f5b5
[2] https://github.com/grpc/grpc/commit/88b1244fd43e81860baa60cc7fb3945a2cca0d11
[3] https://bugzilla.suse.com/show_bug.cgi?id=1228919

References:
https://nvd.nist.gov/vuln/detail/CVE-2024-7246

Signed-off-by: Sudhir Dumbhare <sudumbha@cisco.com>
Signed-off-by: Anuj Mittal <anuj.mittal@oss.qualcomm.com>
This commit is contained in:
Sudhir Dumbhare
2026-06-01 23:39:12 -07:00
committed by Anuj Mittal
parent 8d7e7fa162
commit bc70f00d38
2 changed files with 207 additions and 0 deletions
@@ -0,0 +1,206 @@
From bb1f5e7687799b6650e6b03b285576d9c72dc7d0 Mon Sep 17 00:00:00 2001
From: Craig Tiller <ctiller@google.com>
Date: Thu, 1 Aug 2024 13:02:40 -0700
Subject: [PATCH] [v1.62] [chttp2] Fix a bug in hpack error handling (#37363)
PiperOrigin-RevId: 657234128
PiperOrigin-RevId: 658458047
CVE: CVE-2024-7246
Upstream-Status: Backport [https://github.com/grpc/grpc/commit/1d172cfca56440889ca32ae516b8c2767321f5b5]
Backport Changes:
- The following files were omitted as they are not present in the current version:
test/core/transport/chttp2/hpack_parser_test.cc, introduced in v1.8.0-pre2 by
https://github.com/grpc/grpc/commit/34a57d0346afe95e11104462c30dc468b0cb0b89
test/core/transport/chttp2/hpack_sync_fuzzer.cc and
test/core/transport/chttp2/hpack_sync_fuzzer.proto, introduced in v1.56.0-pre1 by
https://github.com/grpc/grpc/commit/65a2a895afaf1d2072447b9baf246374b182a946
(cherry picked from commit 1d172cfca56440889ca32ae516b8c2767321f5b5)
Signed-off-by: Sudhir Dumbhare <sudumbha@cisco.com>
---
.../chttp2/transport/hpack_parser.cc | 63 +++++++++++--------
.../transport/chttp2/transport/hpack_parser.h | 2 +
2 files changed, 38 insertions(+), 27 deletions(-)
diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
index 31bf46456f..f2fe80c504 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 5edb34b960..f7002186f7 100644
--- a/src/core/ext/transport/chttp2/transport/hpack_parser.h
+++ b/src/core/ext/transport/chttp2/transport/hpack_parser.h
@@ -238,6 +238,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
@@ -13,6 +13,7 @@ SRC_URI += "file://0001-Include-missing-cstdint-header.patch \
file://0001-target.h-define-proper-macro-for-ppc-ppc64.patch \
file://0001-PR-1644-unscaledcycleclock-remove-RISC-V-support.patch \
file://CVE-2024-11407.patch \
file://CVE-2024-7246.patch \
"
SRC_URI[sha256sum] = "c77618071d96b7a8be2c10701a98537823b9c65ba256c0b9067e0594cdbd954d"