Merge remote-tracking branch 'agent/benma-agent/validate-backup-seed-length'
What the developer wrote
Authored by Marko Bencun
Merge remote-tracking branch 'agent/benma-agent/validate-backup-seed-length'
What changed, and why it matters
This commit adds a safety check to the BitBox02 hardware wallet's backup loading code. It now rejects backup files that claim to contain a 'seed' longer than 32 bytes. Without this check, a tampered or malformed backup could potentially cause memory corruption or unexpected behavior when the seed is later used. The change is defensive and includes a new test to confirm oversized seed lengths are rejected.
Treat as a security hardening fix. Review whether other deserialized length fields in the backup and protobuf parsing paths have similar validation, and consider backporting to supported firmware branches. No immediate incident response is indicated by the diff alone.
Security signals we found
Added input validation on deserialized seed_length field
Bounds check prevents oversized seed length (>32 bytes) from being accepted
New unit test covers malformed backup with seed_length mismatch
Defensive hardening of backup deserialization path
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 deserializing backup content. If it exceeds 32, the function returns Err(()). A unit test test_load_from_buffer_oversized_seed_length was added that constructs a BackupV1 protobuf with seed_length=33 and seed=vec![0;32], then asserts that load_from_buffer() fails. The patch is a bounds check on a deserialized length field before downstream use.
Changed components
src/rust/bitbox02-rust/src/backup.rsBackup loading/deserialization pathBackupData seed_length handlingInspect captured patch +34 / −0
### 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 59/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.