What the developer wrote
Authored by benma's agent
btc: reject oversized policy keys
Reject policy keys whose rendered confirmation exceeds the UI label limit.
This prevents registration from approving a key that cannot be displayed in full.
Add boundary coverage for a 640-byte key and rejection at 641 bytes.
What changed, and why it matters
This commit fixes a user-interface safety issue in the BitBox02 hardware wallet's Bitcoin multi-signature policy registration. Previously, a very long policy key could be approved by the user even though the device could not display the full key on its screen. The change now rejects keys whose on-screen confirmation text exceeds the device's maximum label size, preventing a scenario where a user might unknowingly approve a key they cannot fully verify.
Treat as a security-hardening fix and include in the next firmware release. Review whether other rendered confirmation strings in the Bitcoin and Ethereum flows have similar length checks. No immediate CVE action is required unless the vendor identifies an exploitable attack chain.
Security signals we found
Input validation added to enforce UI display limit
Prevents registration of keys that cannot be fully displayed
Boundary test added at MAX_CONFIRM_BODY_SIZE and MAX_CONFIRM_BODY_SIZE+1
Returns InvalidInput error on oversized key rendering
Evidence from the diff
In src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs, the ParsedPolicy::confirm method now checks the rendered length of each key string (key_str) against confirm::MAX_CONFIRM_BODY_SIZE before presenting it to the user. If the rendered confirmation text exceeds this limit, the method returns Error::InvalidInput, aborting registration. The patch also adds a unit test covering a 640-byte key (accepted) and a 641-byte key (rejected). The issue is a UI truncation/truncation-approval risk: an attacker could supply an unusually long xpub derivation path or fingerprint formatting that renders a key string longer than the screen can display, potentially hiding malicious key material from the user during policy registration.
Changed components
src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rsParsedPolicy::confirmBitcoin policy registration flowInspect captured patch +43 / −0
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
index c66d4e7..1081513 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/policies.rs
@@ -490,6 +490,9 @@ impl ParsedPolicy<'_> {
} else if Some(i) == taproot_unspendable_internal_key_index {
key_str = format!("Provably unspendable: {}", key_str)
}
+ if key_str.len() > confirm::MAX_CONFIRM_BODY_SIZE {
+ return Err(Error::InvalidInput);
+ }
hal.ui()
.confirm(&ConfirmParams {
title: &format!("Key {}/{}", i + 1, num_keys),
@@ -1362,6 +1365,46 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_confirm_rejects_oversized_key() {
+ mock_unlocked();
+
+ let coin = BtcCoin::Tbtc;
+ let params = super::super::params::get(coin);
+ let our_key = make_our_key(KEYPATH_ACCOUNT).await;
+
+ for (last_path_element, expected_len, expected_ok) in [
+ (HARDENED, confirm::MAX_CONFIRM_BODY_SIZE, true),
+ (HARDENED + 10, confirm::MAX_CONFIRM_BODY_SIZE + 1, false),
+ ] {
+ let mut external_key = make_key(SOME_XPUB_1);
+ external_key.root_fingerprint = vec![0x12, 0x34, 0x56, 0x78];
+ external_key.keypath = vec![u32::MAX; 43];
+ external_key.keypath.push(last_path_element);
+ let key_str = format!(
+ "[{}/{}]{}",
+ hex::encode(&external_key.root_fingerprint),
+ util::bip32::to_string_no_prefix(&external_key.keypath),
+ bip32::Xpub::from(external_key.xpub.as_ref().unwrap())
+ .serialize_str(bip32::XPubType::Tpub)
+ .unwrap(),
+ );
+ assert_eq!(key_str.len(), expected_len);
+ let policy = make_policy(
+ "wsh(or_b(pk(@0/**),s:pk(@1/**)))",
+ &[external_key, our_key.clone()],
+ );
+ let mut hal = crate::hal::testing::TestingHal::new();
+ let parsed = parse(&mut hal, &policy, coin).await.unwrap();
+
+ let result = parsed
+ .confirm(&mut hal, "Register", params, "Test", Mode::Advanced)
+ .await;
+ assert_eq!(result.is_ok(), expected_ok);
+ assert_eq!(hal.ui.screens.len(), if expected_ok { 5 } else { 3 });
+ }
+ }
+
#[async_test::test]
async fn test_parse_check_dups_in_policy_tr() {
mock_unlocked();
Why this scored 44/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.