Merge "protozero: Avoid DCHECK when parsing malformed packet repeated"

2 files changed, 23 insertions, 1 deletions

diff --git a/include/perfetto/protozero/proto_decoder.h b/include/perfetto/protozero/proto_decoder.h
index 2210532f3..c27fcadf2 100644
--- a/include/perfetto/protozero/proto_decoder.h
+++ b/include/perfetto/protozero/proto_decoder.h
@@ -340,7 +340,8 @@ class PERFETTO_EXPORT_COMPONENT TypedProtoDecoderBase : public ProtoDecoder {
       uint32_t field_id,
       bool* parse_error_location) const {
     const Field& field = Get(field_id);
-    if (field.valid()) {
+    if (field.valid() &&
+        field.type() == proto_utils::ProtoWireType::kLengthDelimited) {
       return PackedRepeatedFieldIterator<wire_type, cpp_type>(
           field.data(), field.size(), parse_error_location);
     }
diff --git a/src/protozero/proto_decoder_unittest.cc b/src/protozero/proto_decoder_unittest.cc
index 0991f887b..1e962d7a6 100644
--- a/src/protozero/proto_decoder_unittest.cc
+++ b/src/protozero/proto_decoder_unittest.cc
@@ -592,5 +592,26 @@ TEST(ProtoDecoderTest, OneBigFieldIdOnly) {
   ASSERT_FALSE(field.valid());
 }
 
+// Check what happens when trying to parse packed repeated field and finding a
+// mismatching wire type instead. A compliant protobuf decoder should accept it,
+// but protozero doesn't handle that. At least it shouldn't crash.
+TEST(ProtoDecoderTest, PacketRepeatedWireTypeMismatch) {
+  protozero::HeapBuffered<pbtest::PackedRepeatedFields> message;
+  // A proper packed encoding should have a length delimited wire type. Use a
+  // var int wire type instead.
+  constexpr int kFieldId = pbtest::PackedRepeatedFields::kFieldInt32FieldNumber;
+  message->AppendTinyVarInt(kFieldId, 5);
+  auto data = message.SerializeAsArray();
+
+  pbtest::PackedRepeatedFields::Decoder decoder(data.data(), data.size());
+  bool parse_error = false;
+  auto it = decoder.field_int32(&parse_error);
+  // The decoder doesn't return a parse error (maybe it should, but that has
+  // been the behavior since the beginning).
+  ASSERT_FALSE(parse_error);
+  // But the iterator returns 0 elements.
+  EXPECT_FALSE(it);
+}
+
 }  // namespace
 }  // namespace protozero