Fixed fuzz bug in upb.

Extending a MessageSet with a non-message extension was causing crashes that would manifest in various ways.

PiperOrigin-RevId: 472496259

upb/fuzz_test_util.cc

@@ -111,8 +111,6 @@ void Builder::BuildEnums() {
 
 bool Builder::LinkExtension(upb_MiniTable_Extension* ext) {
   upb_MiniTable_Field* field = &ext->field;
-  ext->extendee = NextMiniTable();
-  if (!ext->extendee) return false;
   if (field->descriptortype == kUpb_FieldType_Message ||
       field->descriptortype == kUpb_FieldType_Group) {
     auto mt = NextMiniTable();
@@ -140,8 +138,10 @@ void Builder::BuildExtensions(upb_ExtensionRegistry** exts) {
       upb_MiniTable_Extension* ext = reinterpret_cast<upb_MiniTable_Extension*>(
           upb_Arena_Malloc(arena_, sizeof(*ext)));
       upb_MiniTable_Sub sub;
-      ptr =
-          upb_MiniTable_BuildExtension(ptr, end - ptr, ext, sub, status.ptr());
+      const upb_MiniTable* extendee = NextMiniTable();
+      if (!extendee) break;
+      ptr = upb_MiniTable_BuildExtension(ptr, end - ptr, ext, extendee, sub,
+                                         status.ptr());
       if (!ptr) break;
       if (!LinkExtension(ext)) continue;
       if (_upb_extreg_get(*exts, ext->extendee, ext->field.number)) continue;

upb/mini_table.c

@@ -1102,6 +1102,7 @@ upb_MiniTable_Enum* upb_MiniTable_BuildEnum(const char* data, size_t len,
 
 const char* upb_MiniTable_BuildExtension(const char* data, size_t len,
                                          upb_MiniTable_Extension* ext,
+                                         const upb_MiniTable* extendee,
                                          upb_MiniTable_Sub sub,
                                          upb_Status* status) {
   upb_MtDecoder decoder = {
@@ -1117,9 +1118,25 @@ const char* upb_MiniTable_BuildExtension(const char* data, size_t len,
   uint16_t count = 0;
   const char* ret =
       upb_MtDecoder_Parse(&decoder, data, len, ext, sizeof(*ext), &count, NULL);
-  ext->field.mode |= kUpb_LabelFlags_IsExtension;
-  ext->field.offset = 0;
-  ext->field.presence = 0;
+  if (!ret) return NULL;
+
+  upb_MiniTable_Field* f = &ext->field;
+
+  f->mode |= kUpb_LabelFlags_IsExtension;
+  f->offset = 0;
+  f->presence = 0;
+
+  if (extendee->ext & kUpb_ExtMode_IsMessageSet) {
+    // Extensions of MessageSet must be messages.
+    if (!upb_IsSubMessage(f)) return NULL;
+
+    // Extensions of MessageSet must be non-repeating.
+    if ((f->mode & kUpb_FieldMode_Mask) == kUpb_FieldMode_Array) return NULL;
+  }
+
+  ext->extendee = extendee;
+  ext->sub = sub;
+
   return ret;
 }
 

upb/mini_table.h

@@ -156,6 +156,7 @@ void upb_MiniTable_SetSubEnum(upb_MiniTable* table, upb_MiniTable_Field* field,
 
 const char* upb_MiniTable_BuildExtension(const char* data, size_t len,
                                          upb_MiniTable_Extension* ext,
+                                         const upb_MiniTable* extendee,
                                          upb_MiniTable_Sub sub,
                                          upb_Status* status);
 

upb/msg_test.cc

@@ -534,4 +534,40 @@ TEST(MessageTest, MapField) {
 //       "\010\002", 342248070, -806315555);
 // }
 //
+// TEST(FuzzTest, DecodeExtendMessageSetWithNonMessage) {
+//   DecodeEncodeArbitrarySchemaAndPayload(
+//       {{"\n"}, {""}, ".\244", {}}, "\013\032\005\212a#\365\336\020\001\226",
+//       14803219, 670718349);
+// }
+//
+// TEST(FuzzTest, DecodeExtendMessageSetWithNonMessage2) {
+//   DecodeEncodeArbitrarySchemaAndPayload({{"\n", "G", "\n", "\274", ""},
+//                                          {"", "\030"},
+//                                          "_@",
+//                                          {4294967295, 2147483647}},
+//                                         std::string("\013\032\000\220", 4),
+//                                         279975758, 1647495141);
+// }
+//
+// TEST(FuzzTest, DecodeExtendMessageSetWithNonMessage3) {
+//   DecodeEncodeArbitrarySchemaAndPayload(
+//       {{"\n"}, {"B", ""}, "\212:b", {11141121}},
+//       "\013\032\004\357;7\363\020\001\346\240\200\201\271", 399842149,
+//       -452966025);
+// }
+//
+// TEST(FuzzTest, DecodeExtendMessageSetWithNonMessage4) {
+//   DecodeEncodeArbitrarySchemaAndPayload(
+//       {{"\n", "3\340", "\354"}, {}, "B}G", {4294967295, 4082331310}},
+//       "\013\032\004\244B\331\255\020\001\220\224\243\350\t", -561523015,
+//       1683327312);
+// }
+//
+// TEST(FuzzTest, DecodeExtendMessageSetWithNonMessage5) {
+//   DecodeEncodeArbitrarySchemaAndPayload(
+//       {{"\n"}, {""}, "kB", {0}},
+//       "x\203\251\006\013\032\002S\376\010\273\'\020\014\365\207\244\234",
+//       -696925610, -654590577);
+// }
+//
 // end:google_only

upbc/file_layout.cc

@@ -265,8 +265,12 @@ void FilePlatformLayout::BuildExtensions(const protobuf::FileDescriptor* fd) {
                GetFieldModifiers(f));
     upb_MiniTable_Extension& ext = extension_map_[f];
     upb_MiniTable_Sub sub;
+    // The extendee may be from another file, so we build a temporary MiniTable
+    // for it, just for the purpose of building the extension.
+    // Note, we are not caching so this could use more memory than is necessary.
+    upb_MiniTable* extendee = MakeMiniTable(f->containing_type());
     bool ok = upb_MiniTable_BuildExtension(e.data().data(), e.data().size(),
-                                           &ext, sub, status.ptr());
+                                           &ext, extendee, sub, status.ptr());
     if (!ok) {
       // TODO(haberman): Use ABSL CHECK() when it is available.
       fprintf(stderr, "Error building mini-table: %s\n",