From beb01ec89a318203e314e172070ffb5633615701 Mon Sep 17 00:00:00 2001
From: Joshua Haberman <haberman@google.com>
Date: Fri, 24 Jul 2026 10:48:04 -0700
Subject: [PATCH] Fix upb wire decoder accepting field number 0.

Reject field_number 0 tags in _upb_WireReader_SkipValueForceInline.
Protocol Buffer wire format requires field numbers to be positive integers
(1 to 536,870,911). Previously, _upb_WireReader_SkipValueForceInline lacked a
(tag >> 3) == 0 check, allowing upb to accept and round-trip illegal field 0
tags in empty messages, unknown groups, and MessageSet items.

PiperOrigin-RevId: 953447806
---
 upb/wire/decode_test.cc | 55 +++++++++++++++++++++++++++++++++++++++++
 upb/wire/reader.h       |  3 +++
 2 files changed, 58 insertions(+)

diff --git a/upb/wire/decode_test.cc b/upb/wire/decode_test.cc
index 1bf644cd5a842..727eae4069473 100644
--- a/upb/wire/decode_test.cc
+++ b/upb/wire/decode_test.cc
@@ -731,6 +731,61 @@ TEST(DecodeTest, MessageSetConsecutiveUnknowns) {
   }
 }
 
+TEST(DecodeTest, FieldZeroRejected) {
+  Arena mt_arena;
+
+  // 1. Empty message with field 0 varint payload.
+  {
+    upb_MiniTable* empty_mt =
+        (upb_MiniTable*)upb_Arena_Malloc(mt_arena.ptr(), sizeof(upb_MiniTable));
+    memset(empty_mt, 0, sizeof(upb_MiniTable));
+    empty_mt->UPB_PRIVATE(size) = sizeof(upb_Message);
+    empty_mt->UPB_ONLYBITS(field_count) = 0;
+
+    std::string payload("\x00\x00", 2);
+    for (int options : GetDecodeOptionsToTest()) {
+      Arena msg_arena;
+      upb_Message* msg = upb_Message_New(empty_mt, msg_arena.ptr());
+      upb_DecodeStatus result =
+          upb_Decode(payload.data(), payload.size(), msg, empty_mt, nullptr,
+                     options, msg_arena.ptr());
+      EXPECT_EQ(result, kUpb_DecodeStatus_Malformed);
+    }
+  }
+
+  // 2. Field 0 varint inside unknown group.
+  {
+    auto [mt, field] = MiniTable::MakeSingleFieldTable<field_types::Int32>(
+        1, kUpb_DecodeFast_Scalar, mt_arena.ptr());
+
+    // Field 2 (StartGroup) containing Field 0 varint.
+    std::string payload("\x13\x00\x00\x14", 4);
+    for (int options : GetDecodeOptionsToTest()) {
+      Arena msg_arena;
+      upb_Message* msg = upb_Message_New(mt, msg_arena.ptr());
+      upb_DecodeStatus result =
+          upb_Decode(payload.data(), payload.size(), msg, mt, nullptr, options,
+                     msg_arena.ptr());
+      EXPECT_EQ(result, kUpb_DecodeStatus_Malformed);
+    }
+  }
+
+  // 3. Field 0 varint inside MessageSet item.
+  {
+    const upb_MiniTable* mset_mt = &upb_0decode_0test__TestMessageSet_msg_init;
+    // Field 1 (StartGroup for MessageSet Item) containing Field 0 varint.
+    std::string payload("\x0b\x00\x00\x0c", 4);
+    for (int options : GetDecodeOptionsToTest()) {
+      Arena msg_arena;
+      upb_Message* msg = upb_Message_New(mset_mt, msg_arena.ptr());
+      upb_DecodeStatus result =
+          upb_Decode(payload.data(), payload.size(), msg, mset_mt, nullptr,
+                     options, msg_arena.ptr());
+      EXPECT_EQ(result, kUpb_DecodeStatus_Malformed);
+    }
+  }
+}
+
 }  // namespace
 
 }  // namespace test
diff --git a/upb/wire/reader.h b/upb/wire/reader.h
index 0a82bd72335b9..0442e2a31df70 100644
--- a/upb/wire/reader.h
+++ b/upb/wire/reader.h
@@ -128,6 +128,9 @@ UPB_INLINE const char* upb_WireReader_SkipGroup(
 UPB_FORCEINLINE const char* _upb_WireReader_SkipValueForceInline(
     const char* ptr, uint32_t tag, int depth_limit,
     upb_EpsCopyInputStream* stream) {
+  if (UPB_UNLIKELY((tag >> 3) == 0)) {
+    return UPB_PRIVATE(upb_EpsCopyInputStream_ReturnError)(stream);
+  }
   switch (upb_WireReader_GetWireType(tag)) {
     case kUpb_WireType_Varint:
       return upb_WireReader_SkipVarint(ptr, stream);