refactor(core): improve internal representation of protobuf definitions
What changed, and why it matters
This commit is a code cleanup that changes how Trezor firmware internally labels 32-bit versus 64-bit integer fields in its protobuf handling. It does not add or remove security checks by itself, but it makes the type system more precise so that future integer-overflow mistakes are harder to introduce. The change is described by the author as a refactor with no changelog entry.
Treat as a hardening/refactor commit. Review whether the new 32-bit/64-bit split is applied consistently across all generated protobuf definitions and whether any existing message definitions rely on implicit 64-bit behavior for fields now tagged as 32-bit. No immediate security patch is indicated by the diff alone.
Security signals we found
Explicit width-aware integer handling in protobuf varint encode/decode
Added overflow checks for 32-bit and 64-bit varint boundaries in unit tests
Refactor of internal type tags; no new user-facing feature
Evidence from the diff
The patch refactors the Rust protobuf layer in Trezor’s core firmware. It splits the previous UVarInt/SVarInt field types into explicit 32-bit and 64-bit variants (UVarInt32, SVarInt32, UVarInt64, SVarInt64) and updates the code generator (pb2py), field-definition decoding (defs.rs), encode/decode paths, and Python unit tests accordingly. The encoder now downcasts to u32/i32 for 32-bit fields and u64/i64 for 64-bit fields before writing the varint. The decoder now explicitly bounds-checks 32-bit values. The tests add boundary cases for max 32-bit and 64-bit varints and overflow rejection.
Changed components
common/protob/pb2pycore/embed/rust/src/protobuf/decode.rscore/embed/rust/src/protobuf/defs.rscore/embed/rust/src/protobuf/encode.rscore/tests/test_trezor.protobuf.pyInspect captured patch +61 / −23
### common/protob/pb2py
@@ -43,15 +43,15 @@ TYPE_NAMES = {
}
FIELD_TYPES_RUST_BLOB = {
- FieldDescriptor.TYPE_UINT64: 0,
FieldDescriptor.TYPE_UINT32: 0,
- FieldDescriptor.TYPE_SINT64: 1,
FieldDescriptor.TYPE_SINT32: 1,
- FieldDescriptor.TYPE_BOOL: 2,
- FieldDescriptor.TYPE_BYTES: 3,
- FieldDescriptor.TYPE_STRING: 4,
- FieldDescriptor.TYPE_ENUM: 5,
- FieldDescriptor.TYPE_MESSAGE: 6,
+ FieldDescriptor.TYPE_UINT64: 2,
+ FieldDescriptor.TYPE_SINT64: 3,
+ FieldDescriptor.TYPE_BOOL: 4,
+ FieldDescriptor.TYPE_BYTES: 5,
+ FieldDescriptor.TYPE_STRING: 6,
+ FieldDescriptor.TYPE_ENUM: 7,
+ FieldDescriptor.TYPE_MESSAGE: 8,
}
INT_TYPES = (
### core/embed/rust/src/protobuf/decode.rs
@@ -232,8 +232,16 @@ impl Decoder {
}
let num = stream.read_uvarint()?;
match field.get_type() {
- FieldType::UVarInt => Ok(num.try_into()?),
- FieldType::SVarInt => {
+ FieldType::UVarInt32 => {
+ let num = u32::try_from(num)?;
+ Ok(num.try_into()?)
+ }
+ FieldType::SVarInt32 => {
+ let signed_int = i32::try_from(zigzag::to_signed(num))?;
+ Ok(signed_int.try_into()?)
+ }
+ FieldType::UVarInt64 => Ok(num.try_into()?),
+ FieldType::SVarInt64 => {
let signed_int = zigzag::to_signed(num);
Ok(signed_int.try_into()?)
}
### core/embed/rust/src/protobuf/defs.rs
@@ -60,13 +60,15 @@ const STATIC_ASSERT_FIELD_DEF_ALIGNMENT: () = {
impl FieldDef {
pub fn get_type(&self) -> FieldType {
match self.ftype() {
- 0 => FieldType::UVarInt,
- 1 => FieldType::SVarInt,
- 2 => FieldType::Bool,
- 3 => FieldType::Bytes,
- 4 => FieldType::String,
- 5 => FieldType::Enum(get_enum(self.enum_or_msg_offset)),
- 6 => FieldType::Msg(get_msg(self.enum_or_msg_offset)),
+ 0 => FieldType::UVarInt32,
+ 1 => FieldType::SVarInt32,
+ 2 => FieldType::UVarInt64,
+ 3 => FieldType::SVarInt64,
+ 4 => FieldType::Bool,
+ 5 => FieldType::Bytes,
+ 6 => FieldType::String,
+ 7 => FieldType::Enum(get_enum(self.enum_or_msg_offset)),
+ 8 => FieldType::Msg(get_msg(self.enum_or_msg_offset)),
_ => unreachable!(),
}
}
@@ -97,8 +99,10 @@ impl FieldDef {
}
pub enum FieldType {
- UVarInt,
- SVarInt,
+ UVarInt32,
+ SVarInt32,
+ UVarInt64,
+ SVarInt64,
Bool,
Bytes,
String,
@@ -112,9 +116,12 @@ pub const PRIMITIVE_TYPE_LENGTH_DELIMITED: u8 = 2;
impl FieldType {
pub fn primitive_type(&self) -> u8 {
match self {
- FieldType::UVarInt | FieldType::SVarInt | FieldType::Bool | FieldType::Enum(_) => {
- PRIMITIVE_TYPE_VARINT
- }
+ FieldType::UVarInt32
+ | FieldType::SVarInt32
+ | FieldType::UVarInt64
+ | FieldType::SVarInt64
+ | FieldType::Bool
+ | FieldType::Enum(_) => PRIMITIVE_TYPE_VARINT,
FieldType::Bytes | FieldType::String | FieldType::Msg(_) => {
PRIMITIVE_TYPE_LENGTH_DELIMITED
}
### core/embed/rust/src/protobuf/encode.rs
@@ -84,11 +84,20 @@ impl Encoder {
value: Obj,
) -> Result<(), Error> {
match field.get_type() {
- FieldType::UVarInt | FieldType::Enum(_) => {
+ FieldType::UVarInt32 | FieldType::Enum(_) => {
+ let uint = u32::try_from(value)?;
+ stream.write_uvarint(uint.into())?;
+ }
+ FieldType::SVarInt32 => {
+ let sint = i32::try_from(value)?;
+ let uint = zigzag::to_unsigned(i64::from(sint));
+ stream.write_uvarint(uint)?;
+ }
+ FieldType::UVarInt64 => {
let uint = u64::try_from(value)?;
stream.write_uvarint(uint)?;
}
- FieldType::SVarInt => {
+ FieldType::SVarInt64 => {
let sint = i64::try_from(value)?;
let uint = zigzag::to_unsigned(sint);
stream.write_uvarint(uint)?;
### core/tests/test_trezor.protobuf.py
@@ -67,13 +67,27 @@ def test_dump_uvarint(self):
with self.assertRaises(OverflowError):
dump_uvarint(-1)
+ with self.assertRaises(OverflowError):
+ dump_uvarint32(0xFFFFFFFF + 1)
+
def test_load_uvarint(self):
for load_uvarint in (load_uvarint32, load_uvarint64):
self.assertEqual(load_uvarint(b"\x00"), 0)
self.assertEqual(load_uvarint(b"\x01"), 1)
self.assertEqual(load_uvarint(b"\xff\x01"), 0xFF)
self.assertEqual(load_uvarint(b"\xc0\xc4\x07"), 123456)
+ self.assertEqual(load_uvarint32(b"\xff\xff\xff\xff\x0f"), 0xFFFFFFFF)
+ self.assertEqual(
+ load_uvarint64(b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\x01"),
+ 0xFFFFFFFFFFFFFFFF,
+ )
+
+ with self.assertRaises(OverflowError):
+ load_uvarint32(b"\xff\xff\xff\xff\x10")
+ with self.assertRaises(OverflowError):
+ load_uvarint64(b"\xff\xff\xff\xff\xff\xff\xff\xff\xff\x02")
+
def test_reject_malformed_field_key(self):
with self.assertRaises(OverflowError):
load_message(ApplySettings, b"\x80" * 9 + b"\x02")Why this scored 30/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.