Check invalid type_id after casting it to uint32_t.

PiperOrigin-RevId: 536859974
protocolbuffers · May 31, 2023 · d890126 · d890126
1 parent 96c5923
commit d890126
Show file tree

Hide file tree

Showing 13 changed files with 75 additions and 60 deletions.
diff --git a/objectivec/GPBAny.pbobjc.m b/objectivec/GPBAny.pbobjc.m
diff --git a/objectivec/GPBApi.pbobjc.m b/objectivec/GPBApi.pbobjc.m
diff --git a/objectivec/GPBDuration.pbobjc.m b/objectivec/GPBDuration.pbobjc.m
diff --git a/objectivec/GPBEmpty.pbobjc.m b/objectivec/GPBEmpty.pbobjc.m
diff --git a/objectivec/GPBFieldMask.pbobjc.m b/objectivec/GPBFieldMask.pbobjc.m
diff --git a/objectivec/GPBSourceContext.pbobjc.m b/objectivec/GPBSourceContext.pbobjc.m
diff --git a/objectivec/GPBStruct.pbobjc.m b/objectivec/GPBStruct.pbobjc.m
diff --git a/objectivec/GPBTimestamp.pbobjc.m b/objectivec/GPBTimestamp.pbobjc.m
diff --git a/objectivec/GPBType.pbobjc.m b/objectivec/GPBType.pbobjc.m
diff --git a/objectivec/GPBWrappers.pbobjc.m b/objectivec/GPBWrappers.pbobjc.m
diff --git a/src/google/protobuf/extension_set_inl.h b/src/google/protobuf/extension_set_inl.h
@@ -215,13 +215,14 @@ const char* ExtensionSet::ParseMessageSetItemTmpl(
     if (tag == WireFormatLite::kMessageSetTypeIdTag) {
       uint64_t tmp;
       ptr = ParseBigVarint(ptr, &tmp);
-      // We should fail parsing if type id is 0.
-      GOOGLE_PROTOBUF_PARSER_ASSERT(ptr != nullptr && tmp != 0);
+      // We should fail parsing if type id is 0 after cast to uint32.
+      GOOGLE_PROTOBUF_PARSER_ASSERT(ptr != nullptr &&
+                                     static_cast<uint32_t>(tmp) != 0);
       if (state == State::kNoTag) {
-        type_id = tmp;
+        type_id = static_cast<uint32_t>(tmp);
         state = State::kHasType;
       } else if (state == State::kHasPayload) {
-        type_id = tmp;
+        type_id = static_cast<uint32_t>(tmp);
         ExtensionInfo extension;
         bool was_packed_on_wire;
         if (!FindExtension(2, type_id, extendee, ctx, &extension,

diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc
@@ -671,13 +671,14 @@ struct WireFormat::MessageSetParser {
       if (tag == WireFormatLite::kMessageSetTypeIdTag) {
         uint64_t tmp;
         ptr = ParseBigVarint(ptr, &tmp);
-        // We should fail parsing if type id is 0.
-        GOOGLE_PROTOBUF_PARSER_ASSERT(ptr != nullptr && tmp != 0);
+        // We should fail parsing if type id is 0 after cast to uint32.
+        GOOGLE_PROTOBUF_PARSER_ASSERT(ptr != nullptr &&
+                                       static_cast<uint32_t>(tmp) != 0);
         if (state == State::kNoTag) {
-          type_id = tmp;
+          type_id = static_cast<uint32_t>(tmp);
           state = State::kHasType;
         } else if (state == State::kHasPayload) {
-          type_id = tmp;
+          type_id = static_cast<uint32_t>(tmp);
           const FieldDescriptor* field;
           if (ctx->data().pool == nullptr) {
             field = reflection->FindKnownExtensionByNumber(type_id);

diff --git a/src/google/protobuf/wire_format_unittest.inc b/src/google/protobuf/wire_format_unittest.inc
@@ -604,6 +604,19 @@ TEST(WireFormatTest, MessageSetInvalidTypeId) {
   EXPECT_FALSE(message.ParseFromArray(encoded, sizeof(encoded)));
 }
 
+TEST(WireFormatTest, MessageSetNonCanonInvalidTypeId) {
+  // "type_id" is 0 and should fail to parse. uint8_t is used to silence
+  // complaints about narrowing conversion.
+  const uint8_t encoded[] = {
+      013,                               // 1: SGROUP
+      032, 0,                            // 3:LEN 0
+      020, 0x80, 0x80, 0x80, 0x80, 020,  // 2: long-form:2 0
+      014                                // 1: EGROUP
+  };
+  PROTO2_WIREFORMAT_UNITTEST::TestMessageSet message;
+  EXPECT_FALSE(message.ParseFromArray(encoded, sizeof(encoded)));
+}
+
 namespace {
 std::string BuildMessageSetItemStart() {
   std::string data;