CVE-2023-1428: Reachable Assertion (CWE-617)
value: There exists an vulnerability causing an abort() to be called in gRPC. 
The following headers cause gRPC's C++ implementation to abort() when called via http2:

te: x (x != trailers)

:scheme: x (x != http, https)

grpclb_client_stats: x (x == anything)

On top of sending one of those headers, a later header must be sent that gets the total header size past 8KB. We recommend upgrading past git commit 2485fa94bd8a723e5c977d55a3ce10b301b437f8 or v1.53 and above.


CWE description: The product contains an assert() or similar statement that can be triggered by an attacker, which leads to an application exit or other behavior that is more severe than necessary.
CVSS v3 base score: 7.5
Published: 2023-06-09T11:15Z
Repository: https://github.com/grpc/grpc
Language: C++
Files changed: src/core/ext/transport/chttp2/transport/hpack_parser.cc, src/core/lib/transport/metadata_batch.cc, src/core/lib/transport/metadata_batch.h, test/core/transport/chttp2/hpack_parser_corpus/clusterfuzz-testcase-minimized-hpack_parser_fuzzer-4859070937169920
Commit date: 2023-03-01 09:17:49 -0800
Commit message: [chttp2] Fix fuzzer found bug (#32507)

<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
Diff: diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
index d10ff72847..c849f39931 100644
--- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc
+++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc
@@ -801,11 +801,11 @@ class HPackParser::Parser {
       AddToSummary(key.as_string_view(), value.size());
     }
 
     template <typename Key, typename Value>
     void Encode(Key, const Value& value) {
-      AddToSummary(Key::key(), Key::Encode(value).size());
+      AddToSummary(Key::key(), EncodedSizeOfKey(Key(), value));
     }
 
    private:
     void AddToSummary(absl::string_view key,
                       size_t value_length) GPR_ATTRIBUTE_NOINLINE {
diff --git a/src/core/lib/transport/metadata_batch.cc b/src/core/lib/transport/metadata_batch.cc
index 27ea2e77b8..1150e41b81 100644
--- a/src/core/lib/transport/metadata_batch.cc
+++ b/src/core/lib/transport/metadata_batch.cc
@@ -166,10 +166,21 @@ StaticSlice HttpSchemeMetadata::Encode(ValueType x) {
     default:
       abort();
   }
 }
 
+size_t EncodedSizeOfKey(HttpSchemeMetadata, HttpSchemeMetadata::ValueType x) {
+  switch (x) {
+    case HttpSchemeMetadata::kHttp:
+      return 4;
+    case HttpSchemeMetadata::kHttps:
+      return 5;
+    default:
+      return 0;
+  }
+}
+
 const char* HttpSchemeMetadata::DisplayValue(ValueType content_type) {
   switch (content_type) {
     case kHttp:
       return "http";
     case kHttps:
diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h
index 635eea01c8..5b4f6d972f 100644
--- a/src/core/lib/transport/metadata_batch.h
+++ b/src/core/lib/transport/metadata_batch.h
@@ -47,10 +47,20 @@
 #include "src/core/lib/slice/slice.h"
 #include "src/core/lib/transport/parsed_metadata.h"
 
 namespace grpc_core {
 
+// Given a metadata key and a value, return the encoded size.
+// Defaults to calling the key's Encode() method and then calculating the size
+// of that, but can be overridden for specific keys if there's a better way of
+// doing this.
+// May return 0 if the size is unknown/unknowable.
+template <typename Key>
+size_t EncodedSizeOfKey(Key, const typename Key::ValueType& value) {
+  return Key::Encode(value).size();
+}
+
 // grpc-timeout metadata trait.
 // ValueType is defined as Timestamp - an absolute timestamp (i.e. a
 // deadline!), that is converted to a duration by transports before being
 // sent.
 // TODO(ctiller): Move this elsewhere. During the transition we need to be able
@@ -88,10 +98,14 @@ struct TeMetadata {
   }
   static const char* DisplayValue(ValueType te);
   static const char* DisplayMemento(MementoType te) { return DisplayValue(te); }
 };
 
+inline size_t EncodedSizeOfKey(TeMetadata, TeMetadata::ValueType x) {
+  return x == TeMetadata::kTrailers ? 8 : 0;
+}
+
 // content-type metadata trait.
 struct ContentTypeMetadata {
   static constexpr bool kRepeatable = false;
   // gRPC says that content-type can be application/grpc[;something]
   // Core has only ever verified the prefix.
@@ -138,10 +152,12 @@ struct HttpSchemeMetadata {
   static const char* DisplayMemento(MementoType content_type) {
     return DisplayValue(content_type);
   }
 };
 
+size_t EncodedSizeOfKey(HttpSchemeMetadata, HttpSchemeMetadata::ValueType x);
+
 // method metadata trait.
 struct HttpMethodMetadata {
   static constexpr bool kRepeatable = false;
   enum ValueType : uint8_t {
     kPost,
@@ -360,10 +376,15 @@ struct GrpcLbClientStatsMetadata {
   static MementoType ParseMemento(Slice, MetadataParseErrorFn) {
     return nullptr;
   }
 };
 
+inline size_t EncodedSizeOfKey(GrpcLbClientStatsMetadata,
+                               GrpcLbClientStatsMetadata::ValueType) {
+  return 0;
+}
+
 // lb-token metadata
 struct LbTokenMetadata : public SimpleSliceBasedMetadata {
   static constexpr bool kRepeatable = false;
   static absl::string_view key() { return "lb-token"; }
 };
diff --git a/test/core/transport/chttp2/hpack_parser_corpus/clusterfuzz-testcase-minimized-hpack_parser_fuzzer-4859070937169920 b/test/core/transport/chttp2/hpack_parser_corpus/clusterfuzz-testcase-minimized-hpack_parser_fuzzer-4859070937169920
new file mode 100644
index 0000000000..8a0a52d7b4
--- /dev/null
+++ b/test/core/transport/chttp2/hpack_parser_corpus/clusterfuzz-testcase-minimized-hpack_parser_fuzzer-4859070937169920
@@ -0,0 +1,4 @@
+frames {
+  max_metadata_length: 58
+  parse: "@\002te\000@\002te\002@\000et\000;e"
+}

Vulnerable code:       AddToSummary(Key::key(), Key::Encode(value).size());
Fixed code:       AddToSummary(Key::key(), EncodedSizeOfKey(Key(), value));
size_t EncodedSizeOfKey(HttpSchemeMetadata, HttpSchemeMetadata::ValueType x) {
  switch (x) {
    case HttpSchemeMetadata::kHttp:
      return 4;
    case HttpSchemeMetadata::kHttps:
      return 5;
    default:
      return 0;
  }
}

// Given a metadata key and a value, return the encoded size.
// Defaults to calling the key's Encode() method and then calculating the size
// of that, but can be overridden for specific keys if there's a better way of
// doing this.
// May return 0 if the size is unknown/unknowable.
template <typename Key>
size_t EncodedSizeOfKey(Key, const typename Key::ValueType& value) {
  return Key::Encode(value).size();
}

inline size_t EncodedSizeOfKey(TeMetadata, TeMetadata::ValueType x) {
  return x == TeMetadata::kTrailers ? 8 : 0;
}

size_t EncodedSizeOfKey(HttpSchemeMetadata, HttpSchemeMetadata::ValueType x);

inline size_t EncodedSizeOfKey(GrpcLbClientStatsMetadata,
                               GrpcLbClientStatsMetadata::ValueType) {
  return 0;
}

frames {
  max_metadata_length: 58
  parse: "@\002te\000@\002te\002@\000et\000;e"
}