What the developer wrote
Authored by benma's agent
backup: validate decoded seed length
Reject backup data whose declared seed length exceeds the fixed
32-byte seed field. This prevents malformed SD-card backups from
triggering an out-of-bounds panic while listing backups.
Add a regression test with an otherwise valid protobuf and recomputed
checksum.
Tests:
- `cargo test --manifest-path src/rust/Cargo.toml --all-features -- --test-threads 1`
- `./scripts/dev_exec.sh make run-unit-tests`
What changed, and why it matters
This update fixes a bug in how the BitBox02 hardware wallet reads backup files from an SD card. A tampered backup file could claim to contain a seed longer than the 32-byte limit, which previously caused the device to panic (crash) when listing backups. The fix rejects such malformed files, and a new test confirms the behavior.
Apply the patch and run the included Rust unit tests and device unit tests. Consider auditing other protobuf-decoded length fields for similar missing bounds checks.
Security signals we found
Out-of-bounds/panic condition in backup parsing
Missing input validation on decoded protobuf field
SD-card backup file could be attacker-controlled
Regression test added for malformed seed length
Evidence from the diff
In src/rust/bitbox02-rust/src/backup.rs, the load_from_buffer function now validates that backup_data.0.seed_length is not greater than 32 after decoding a backup’s protobuf data. Before this change, a malformed backup with seed_length > 32 could trigger an out-of-bounds panic during backup listing. A regression test constructs a valid protobuf backup with seed_length: 33 and a recomputed checksum, asserting that load_from_buffer returns an error.
Changed components
src/rust/bitbox02-rust/src/backup.rsBackup loading / listing functionalitySD-card backup parserInspect captured patch +34 / −0
diff --git a/src/rust/bitbox02-rust/src/backup.rs b/src/rust/bitbox02-rust/src/backup.rs
index 4fdf1f1..6fd4b51 100644
--- a/src/rust/bitbox02-rust/src/backup.rs
+++ b/src/rust/bitbox02-rust/src/backup.rs
@@ -117,6 +117,9 @@ fn load_from_buffer(buf: &[u8]) -> Result<(Zeroizing<BackupData>, pb_backup::Bac
})) => {
let mut backup_data: Zeroizing<BackupData> = Default::default();
backup_data.0.merge(content.data.as_slice()).or(Err(()))?;
+ if backup_data.0.seed_length > 32 {
+ return Err(());
+ }
let checksum = compute_checksum(
content.metadata.as_ref().unwrap(),
@@ -319,6 +322,37 @@ mod tests {
assert_eq!(id(&seed_32), expected_output_32);
}
+ #[test]
+ fn test_load_from_buffer_oversized_seed_length() {
+ let data = pb_backup::BackupData {
+ seed_length: 33,
+ seed: vec![0; 32],
+ birthdate: 0,
+ generator: String::new(),
+ };
+ let metadata = pb_backup::BackupMetaData {
+ timestamp: 0,
+ name: String::new(),
+ mode: pb_backup::BackupMode::Plaintext as _,
+ };
+ let length = 0;
+ let checksum = compute_checksum(&metadata, &data, length).unwrap();
+ let backup = pb_backup::Backup {
+ backup_version: Some(pb_backup::backup::BackupVersion::BackupV1(
+ pb_backup::BackupV1 {
+ content: Some(pb_backup::BackupContent {
+ checksum,
+ metadata: Some(metadata),
+ length,
+ data: data.encode_to_vec(),
+ }),
+ },
+ )),
+ };
+
+ assert!(load_from_buffer(&backup.encode_to_vec()).is_err());
+ }
+
async fn _test_create_load(seed: &[u8]) {
let mut mock_hal = TestingHal::new();
let timestamp = 1601281809;
Why this scored 60/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.