fix(core): avoid panic on sentinel glyph lookup
What the developer wrote
Authored by obrusvit
fix(core): avoid panic on sentinel glyph lookup
[no changelog]
What changed, and why it matters
This commit fixes a bug in the Trezor firmware's translation system where looking up a special 'sentinel' (end-marker) value could cause the device to crash (panic). The fix changes the code to safely check whether the next entry exists before reading it, returning 'not found' instead of crashing. A test was added to confirm the sentinel lookup no longer panics.
Treat as a low-severity hardening fix. There is no direct evidence of exploitability for code execution or key extraction, but the panic could be triggered by malformed or attacker-controlled translation data. Ensure the fix is included in firmware builds and consider fuzzing the translation blob parser for additional panic conditions.
Security signals we found
Denial-of-service vector: out-of-bounds access leading to panic
Fix pattern: replace unchecked indexing with safe Option-based access
New regression test specifically exercises the panic condition
Evidence from the diff
In core/embed/rust/src/translations/blob.rs, the Table::get method previously indexed self.offsets[idx + 1] unconditionally. When the queried id equals SENTINEL_ID, idx resolves to the last offset entry and idx + 1 is out of bounds, triggering a Rust panic. The patch replaces direct indexing with .get(idx + 1)? so the lookup returns None gracefully. A unit test verifies that get(SENTINEL_ID) returns None and does not panic.
Changed components
core/embed/rust/src/translations/blob.rsTrezor Core firmware translation blob parserTable::get glyph/translation lookupInspect captured patch +23 / −1
### core/embed/rust/src/translations/blob.rs
@@ -196,7 +196,9 @@ impl<'a> Table<'a> {
.ok()
.and_then(|idx| {
let start = self.offsets[idx].offset.into();
- let end = self.offsets[idx + 1].offset.into();
+ // When `id` is the sentinel, `idx` is the last entry and there
+ // is no next offset to read - return None instead of panicking.
+ let end = self.offsets.get(idx + 1)?.offset.into();
self.data.get(start..end)
})
}
@@ -682,4 +684,24 @@ mod tests {
}
assert_eq!(ENGLISH_CHUNK.get(ENGLISH_CHUNK.len()), None);
}
+
+ #[test]
+ fn test_table_get() {
+ // Table layout: u16 count, (count + 1) packed (u16 id, u16 offset)
+ // entries (the last one being the sentinel), then the data.
+ let bytes: &[u8] = &[
+ 2, 0, // entry count
+ 1, 0, 0, 0, // id 1, offset 0
+ 2, 0, 3, 0, // id 2, offset 3
+ 0xFF, 0xFF, 6, 0, // sentinel id, offset 6
+ b'a', b'b', b'c', b'd', b'e', b'f',
+ ];
+ let table = Table::new(InputStream::new(bytes)).expect("valid table");
+ table.validate().expect("valid table");
+ assert_eq!(table.get(1), Some(&b"abc"[..]));
+ assert_eq!(table.get(2), Some(&b"def"[..]));
+ assert_eq!(table.get(3), None);
+ // Asking for the sentinel id must not panic and must return None.
+ assert_eq!(table.get(SENTINEL_ID), None);
+ }
}Why this scored 37/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.