mmo-ref/dep-protobuf
1
0
Fork 0
mirror of https://github.com/protocolbuffers/protobuf synced 2026-08-26 02:23:14 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

check sub array bounds before indexing in upb_MiniTable_Link (#28553)

Browse source 
upb_MiniTable_Link reads sub_tables[msg_count] (and sub_enums[enum_count]) before comparing the index against the caller-supplied length, so a mini table with more message or closed-enum fields than the array holds reads one past the end before the count check fires. This is the too-short-array case the header documents should just return false, and it's reachable whenever the array is sized from a different mini table than the one being linked. Move the length check ahead of the array access at both sites; the regression test builds a two-message-field table and links it against a one-element array, which trips ASAN before the change.

Closes #28553

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/28553 from arshsmith1:minitable-link-bounds 04231a5a6a
PiperOrigin-RevId: 963598486
This commit is contained in:
arshsmith 2026-08-12 12:31:52 -07:00 • committed by Copybara-Service
parent 91b86b0535
commit 085b305c25
2 changed files with 22 additions and 2 deletions
Download patch file Download diff file Expand all files Collapse all files

20
upb/mini_descriptor/internal/encode_test.cc
View file

@ -22,6 +22,7 @@
#include "upb/mini_descriptor/decode.h"
#include "upb/mini_descriptor/internal/base92.h"
#include "upb/mini_descriptor/internal/modifiers.h"
#include "upb/mini_descriptor/link.h"
#include "upb/mini_table/enum.h"
#include "upb/mini_table/field.h"
#include "upb/mini_table/internal/message.h"
@ -299,6 +300,25 @@ TEST(MiniTableTest, SubsInitializedToNull) {
upb_MiniTable_FieldIsLinked(upb_MiniTable_GetFieldByIndex(table, 1)));
}
TEST(MiniTableTest, LinkShortSubTableArray) {
upb::Arena arena;
upb::MtDataEncoder e;
// Message with two message fields, so linking expects two sub-tables.
ASSERT_TRUE(e.StartMessage(0));
ASSERT_TRUE(e.PutField(kUpb_FieldType_Message, 1, 0));
ASSERT_TRUE(e.PutField(kUpb_FieldType_Message, 2, 0));
upb::Status status;
upb_MiniTable* table = upb_MiniTable_Build(e.data().data(), e.data().size(),
arena.ptr(), status.ptr());
ASSERT_NE(nullptr, table);
ASSERT_EQ(upb_MiniTable_FieldCount(table), 2);
// Passing an array shorter than the number of message fields must fail
// cleanly without reading past the end of the array.
const upb_MiniTable* subs[1] = {nullptr};
EXPECT_FALSE(upb_MiniTable_Link(table, subs, 1, nullptr, 0));
}
TEST(MiniTableEnumTest, PositiveAndNegative) {
upb::Arena arena;
upb::MtDataEncoder e;

4
upb/mini_descriptor/link.c
View file

@ -165,8 +165,8 @@ bool upb_MiniTable_Link(upb_MiniTable* m, const upb_MiniTable** sub_tables,
upb_MiniTableField* f =
(upb_MiniTableField*)upb_MiniTable_GetFieldByIndex(m, i);
if (upb_MiniTableField_CType(f) == kUpb_CType_Message) {
if (msg_count >= sub_table_count) return false;
const upb_MiniTable* sub = sub_tables[msg_count++];
if (msg_count > sub_table_count) return false;
if (sub && !upb_MiniTable_SetSubMessage(m, f, sub)) return false;
}
}
@ -175,8 +175,8 @@ bool upb_MiniTable_Link(upb_MiniTable* m, const upb_MiniTable** sub_tables,
upb_MiniTableField* f =
(upb_MiniTableField*)upb_MiniTable_GetFieldByIndex(m, i);
if (upb_MiniTableField_IsClosedEnum(f)) {
if (enum_count >= sub_enum_count) return false;
const upb_MiniTableEnum* sub = sub_enums[enum_count++];
if (enum_count > sub_enum_count) return false;
if (sub && !upb_MiniTable_SetSubEnum(m, f, sub)) return false;
}
}