From 2485fa94bd8a723e5c977d55a3ce10b301b437f8 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 1 Mar 2023 09:17:49 -0800 Subject: [PATCH] [chttp2] Fix fuzzer found bug (#32507) --- .../chttp2/transport/hpack_parser.cc | 2 +- src/core/lib/transport/metadata_batch.cc | 11 ++++++++++ src/core/lib/transport/metadata_batch.h | 21 +++++++++++++++++++ ...mized-hpack_parser_fuzzer-4859070937169920 | 4 ++++ 4 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 test/core/transport/chttp2/hpack_parser_corpus/clusterfuzz-testcase-minimized-hpack_parser_fuzzer-4859070937169920 diff --git a/src/core/ext/transport/chttp2/transport/hpack_parser.cc b/src/core/ext/transport/chttp2/transport/hpack_parser.cc index d10ff728470..c849f39931a 100644 --- a/src/core/ext/transport/chttp2/transport/hpack_parser.cc +++ b/src/core/ext/transport/chttp2/transport/hpack_parser.cc @@ -803,7 +803,7 @@ class HPackParser::Parser { template void Encode(Key, const Value& value) { - AddToSummary(Key::key(), Key::Encode(value).size()); + AddToSummary(Key::key(), EncodedSizeOfKey(Key(), value)); } private: diff --git a/src/core/lib/transport/metadata_batch.cc b/src/core/lib/transport/metadata_batch.cc index 27ea2e77b88..1150e41b81d 100644 --- a/src/core/lib/transport/metadata_batch.cc +++ b/src/core/lib/transport/metadata_batch.cc @@ -168,6 +168,17 @@ StaticSlice HttpSchemeMetadata::Encode(ValueType x) { } } +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: diff --git a/src/core/lib/transport/metadata_batch.h b/src/core/lib/transport/metadata_batch.h index 635eea01c8f..5b4f6d972f7 100644 --- a/src/core/lib/transport/metadata_batch.h +++ b/src/core/lib/transport/metadata_batch.h @@ -49,6 +49,16 @@ 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 +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 @@ -90,6 +100,10 @@ struct TeMetadata { 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; @@ -140,6 +154,8 @@ struct HttpSchemeMetadata { } }; +size_t EncodedSizeOfKey(HttpSchemeMetadata, HttpSchemeMetadata::ValueType x); + // method metadata trait. struct HttpMethodMetadata { static constexpr bool kRepeatable = false; @@ -362,6 +378,11 @@ struct GrpcLbClientStatsMetadata { } }; +inline size_t EncodedSizeOfKey(GrpcLbClientStatsMetadata, + GrpcLbClientStatsMetadata::ValueType) { + return 0; +} + // lb-token metadata struct LbTokenMetadata : public SimpleSliceBasedMetadata { static constexpr bool kRepeatable = false; 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 00000000000..8a0a52d7b47 --- /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" +}