Fix risk of upb itof corruption when legacy json_name conflicts are allowed.

This addresses a risk as identified by security researcher https://github.com/sphl.

Since it is only exposed on untrusted descriptors this is outside of our CVE disclosure model, but we do intend to harden these cases as defense in depth.

PiperOrigin-RevId: 998732157

upb/reflection/message_def.c

diff --git a/upb/reflection/message_def.c b/upb/reflection/message_def.c
index c737d2b..433f4f3 100644
--- a/upb/reflection/message_def.c
+++ b/upb/reflection/message_def.c
@@ -406,10 +406,7 @@
   const char* shortname = upb_FieldDef_Name(f);
   const size_t shortnamelen = strlen(shortname);
 
-  upb_value v = upb_value_constptr(f);
-
-  upb_value existing_v;
-  if (upb_strtable_lookup(&m->ntof, shortname, &existing_v)) {
+  if (upb_strtable_lookup(&m->ntof, shortname, NULL)) {
     _upb_DefBuilder_Errf(ctx, "duplicate field name (%s)", shortname);
   }
 
@@ -424,13 +421,13 @@
   if (!skip_json_conflicts && strcmp(shortname, json_name) != 0 &&
       google_protobuf_FeatureSet_json_format(m->resolved_features) ==
           google_protobuf_FeatureSet_ALLOW &&
-      upb_strtable_lookup(&m->ntof, json_name, &v)) {
+      upb_strtable_lookup(&m->ntof, json_name, NULL)) {
     _upb_DefBuilder_Errf(
         ctx, "duplicate json_name for (%s) with original field name (%s)",
         shortname, json_name);
   }
 
-  if (upb_strtable_lookup(&m->jtof, json_name, &v)) {
+  if (upb_strtable_lookup(&m->jtof, json_name, NULL)) {
     if (!skip_json_conflicts) {
       _upb_DefBuilder_Errf(ctx, "duplicate json_name (%s)", json_name);
     }
@@ -445,7 +442,8 @@
     _upb_DefBuilder_Errf(ctx, "duplicate field number (%u)", field_number);
   }
 
-  ok = upb_inttable_insert(&m->itof, field_number, v, ctx->arena);
+  ok = upb_inttable_insert(&m->itof, field_number, upb_value_constptr(f),
+                           ctx->arena);
   if (!ok) _upb_DefBuilder_OomErr(ctx);
 }
 

upb/reflection/reflection_test.cc

diff --git a/upb/reflection/reflection_test.cc b/upb/reflection/reflection_test.cc
index 635df53..50a387d 100644
--- a/upb/reflection/reflection_test.cc
+++ b/upb/reflection/reflection_test.cc
@@ -498,6 +498,77 @@
   EXPECT_TRUE(status.ok()) << status.message();
 }
 
+TEST(ReflectionTest, LegacyJsonFieldConflictsKeepsCorrectNumberMapping) {
+  // With deprecated_legacy_json_field_conflicts, two fields may share a
+  // json_name. Each field number must still map to its own FieldDef.
+  upb::DefPool pool =
+      LoadDescriptorProto(
+          R"pb(
+            syntax: "proto2"
+            name: "F"
+            message_type { name: "Sub" }
+            message_type {
+              name: "M"
+              options { deprecated_legacy_json_field_conflicts: true }
+              field {
+                name: "a"
+                number: 1
+                label: LABEL_OPTIONAL
+                type: TYPE_MESSAGE
+                type_name: ".Sub"
+                json_name: "j"
+              }
+              field {
+                name: "b"
+                number: 2
+                label: LABEL_OPTIONAL
+                type: TYPE_FIXED64
+                json_name: "j"
+              }
+            }
+          )pb")
+          .value();
+  upb::MessageDefPtr m = pool.FindMessageByName("M");
+  ASSERT_TRUE(m);
+  upb::FieldDefPtr a = m.FindFieldByNumber(1);
+  upb::FieldDefPtr b = m.FindFieldByNumber(2);
+  ASSERT_TRUE(a);
+  ASSERT_TRUE(b);
+  EXPECT_STREQ(a.name(), "a");
+  EXPECT_EQ(a.number(), 1);
+  EXPECT_STREQ(b.name(), "b");
+  EXPECT_EQ(b.number(), 2);
+  EXPECT_EQ(b.type(), kUpb_FieldType_Fixed64);
+}
+
+TEST(ReflectionTest, JsonFieldConflictsRejectedWithoutLegacyOption) {
+  absl::Status status = LoadDescriptorProto(
+                            R"pb(
+                              syntax: "proto2"
+                              name: "F"
+                              message_type {
+                                name: "M"
+                                field {
+                                  name: "a"
+                                  number: 1
+                                  label: LABEL_OPTIONAL
+                                  type: TYPE_FIXED64
+                                  json_name: "j"
+                                }
+                                field {
+                                  name: "b"
+                                  number: 2
+                                  label: LABEL_OPTIONAL
+                                  type: TYPE_FIXED64
+                                  json_name: "j"
+                                }
+                              }
+                            )pb")
+                            .status();
+  EXPECT_THAT(std::string(status.message()),
+              HasSubstr("duplicate json_name (j)"));
+}
+
 TEST(ReflectionTest, DefPoolFindMethodsRespectSize) {
   upb::DefPool pool = LoadDescriptorProto(R"pb(
                         syntax: "proto2"