From 7969ad76fbb3e78323435168742eb098742da23a Mon Sep 17 00:00:00 2001 From: David Sarkisyan Date: Sat, 29 Aug 2026 16:38:38 -0400 Subject: [PATCH] upb: validate message tags before consuming depth Check that a linked message fasttable slot matches the actual tag before looking up the subtable or entering the message path. This keeps mismatched tags on the fallback path and avoids rejecting valid unknown fields at an exact recursion boundary. Signed-off-by: David Sarkisyan --- upb/wire/decode_fast/field_message.c | 17 ++++++++--------- upb/wire/decode_test.cc | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/upb/wire/decode_fast/field_message.c b/upb/wire/decode_fast/field_message.c index 817ccdd4122a7..3d31e061fcd62 100644 --- a/upb/wire/decode_fast/field_message.c +++ b/upb/wire/decode_fast/field_message.c @@ -75,19 +75,18 @@ void upb_DecodeFast_Message(upb_Decoder* d, const char** ptr, upb_Message* msg, table->UPB_ONLYBITS(fields), submsg_ofs, upb_MiniTableSubInternal); const upb_MiniTable* subtablep = sub->UPB_PRIVATE(submsg); + uint16_t expected = upb_DecodeFastData_GetExpectedTag(*data); + uint16_t actual = upb_DecodeFastData2_GetOriginalTag(data2); + if (UPB_UNLIKELY(!upb_DecodeFast_TagMatches(expected, actual, tagsize))) { + UPB_DECODEFAST_EXIT(kUpb_DecodeFastNext_FallbackMismatchedSlot, ret); + return; + } + upb_DecodeFast_MessageContext ctx = {subtablep, card == kUpb_DecodeFast_Repeated}; if (subtablep == NULL) { - // Unlinked messages are treated as unknown fields. Go straight to unknown - // decoder if the tag matches. - uint16_t expected = upb_DecodeFastData_GetExpectedTag(*data); - uint16_t actual = upb_DecodeFastData2_GetOriginalTag(data2); - if (UPB_UNLIKELY(!upb_DecodeFast_TagMatches(expected, actual, tagsize))) { - UPB_DECODEFAST_EXIT(kUpb_DecodeFastNext_FallbackMismatchedSlot, ret); - return; - } - + // Unlinked messages are treated as unknown fields. #ifndef NDEBUG uint16_t case_offset = upb_DecodeFastData_GetCaseOffset(*data); if (case_offset != 0) { diff --git a/upb/wire/decode_test.cc b/upb/wire/decode_test.cc index 2a6cf4994525f..273b8d218e02b 100644 --- a/upb/wire/decode_test.cc +++ b/upb/wire/decode_test.cc @@ -760,6 +760,33 @@ TEST(DecodeTest, DecodeExtensionAsUnknownWithPreexistingUnknown) { &ext_iter)); } +TEST(DecodeTest, MismatchedMessageSlotDoesNotConsumeDepth) { + upb::Arena mt_arena; + + auto [mt, field] = + test::MiniTable::MakeSingleFieldTable( + 1, kUpb_DecodeFast_Scalar, mt_arena.ptr()); + const upb_MiniTable* subs[1] = {mt}; + ASSERT_TRUE( + upb_MiniTable_Link(const_cast(mt), subs, 1, nullptr, 0)); + + // The outer field is a message. Inside it, field 1 has the varint wire type, + // so it is preserved as unknown instead of recursing into another message. + const std::string payload("\x0a\x02\x08\x01", 4); + const int depth_one = upb_Decode_LimitDepth(0, 1); + + for (int extra_options : GetDecodeOptionsToTest()) { + upb::Arena msg_arena; + upb_Message* msg = upb_Message_New(mt, msg_arena.ptr()); + ASSERT_NE(msg, nullptr); + + const upb_DecodeStatus result = + upb_Decode(payload.data(), payload.size(), msg, mt, nullptr, + depth_one | extra_options, msg_arena.ptr()); + EXPECT_EQ(result, kUpb_DecodeStatus_Ok) << upb_DecodeStatus_String(result); + } +} + TEST(DecodeTest, DecodeGroupFieldFromDelimitedWireFormatAsUnknown) { upb::Arena mt_arena; upb::Arena msg_arena;