Public commit record
23/100 · Opaque
What the developer wrote
Authored by Marko Bencun
security improvements
✓ Subject identifies a change✓ Names security-relevant behavior explicitly! Too few words to establish purpose! No meaningful explanatory body! Opaque security-relevant change
What changed, and why it matters
This BitBox02 firmware update is a broad security patch that fixes several independent bugs: it prevents a maliciously oversized USB report from overflowing memory, stops a corrupted Bluetooth pairing database from being read or written with invalid lengths, allows full-size firmware images in the bootloader, hardens how Bitcoin, Cardano, Ethereum, and typed-data (EIP-712) transactions are parsed and shown to the user, and adds a user confirmation before creating backups. The changes are defensive and reduce the chance that an attacker could trick the device into signing something the user did not intend.
Recommended action
Treat this as a security update. Users should upgrade to firmware v9.26.3 and bootloader v1.1.3 when available. Developers should review the individual hardening changes for completeness, especially the Bitcoin output-type checks and the Ethereum streaming threshold, to ensure no edge cases remain unhandled.
Security signals we found
01
Bounds check added to USB HID Set Report input length
02
BLE bond DB length validation hardened against negative and oversized values
03
Bootloader firmware image size limit relaxed to intended maximum
04
Bitcoin transaction output validation tightened for OP_RETURN, silent payments, and payment requests
05
Cardano transaction network-binding check and overflow-hardened arithmetic added
06
Ethereum streaming/non-streaming data length threshold enforced
07
EIP-712 typed message schema and chainId validation added
08
Backup creation now requires explicit on-device user confirmation
09
Backup loading now verifies seed id matches backup directory
010
Cryptographic key material dependencies updated to use zeroize feature
Evidence from the diff
The commit bundles multiple hardening changes. In the bootloader, the firmware chunk limit is corrected from FIRMWARE_MAX_NUM_CHUNKS - 1 to FIRMWARE_MAX_NUM_CHUNKS. In memory_shared.c, BLE bond DB read/write now rejects negative or oversized lengths and clears the shared chunk on invalid reads. The DA14531 handler propagates the set_bond_db failure and treats len as signed correctly. The USB HID Set Report handler now bounds-checks the incoming report length against USB_HID_REPORT_OUT_SIZE. Rust changes include: backup loading now verifies the backup directory matches the seed id; backup creation requires explicit user confirmation; Bitcoin signing rejects OP_RETURN outputs that are marked ours, carry silent-payment addresses, or belong to payment requests, and renames/tightens the change-output accounting; Cardano signing warns when a transaction has no outputs or withdrawals to bind it to a network and adds checked arithmetic for output totals and fee; Ethereum signing rejects streaming requests whose data_length is within the non-streaming threshold, and EIP-712 typed-message signing validates identifiers in the schema, validates/caches the chainId field, and warns when no chainId is present; message verification now allows blank lines but rejects empty messages. Cargo dependencies are updated to enable zeroize for x25519-dalek and ed25519-dalek.
Changed components
src/bootloader/bootloader.csrc/da14531/da14531_handler.csrc/memory/memory_shared.csrc/memory/memory_shared.hsrc/usb/class/hid/hid.csrc/rust/bitbox02-rust/src/backup.rssrc/rust/bitbox02-rust/src/hww/api/backup.rssrc/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rssrc/rust/bitbox02-rust/src/hww/api/cardano/sign_transaction.rssrc/rust/bitbox02-rust/src/hww/api/ethereum/sign.rssrc/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rssrc/rust/bitbox02-rust/src/workflow/verify_message.rssrc/rust/bitbox02-noise/Cargo.tomlsrc/rust/bitbox02-rust/Cargo.tomlInspect captured patch +1117 / −250
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f561e4..83802bb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,9 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
### [Unreleased]
+### v9.26.3
+- Security improvements
+
### v9.26.1
- Fix a payment request validation issue
@@ -198,6 +201,9 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
## Bootloader
+### v1.1.3
+- Allow full sized firmware images
+
### v1.1.2
- BitBox02 Nova: correctly orient bootloader screen
diff --git a/src/bootloader/bootloader.c b/src/bootloader/bootloader.c
index 1455158..2f01efa 100644
--- a/src/bootloader/bootloader.c
+++ b/src/bootloader/bootloader.c
@@ -518,7 +518,7 @@ static size_t _api_write_chunk(const uint8_t* buf, uint8_t chunknum, uint8_t* ou
*/
static size_t _api_firmware_erase(uint8_t firmware_num_chunks, uint8_t* output)
{
- if (firmware_num_chunks > FIRMWARE_MAX_NUM_CHUNKS - 1) {
+ if (firmware_num_chunks > FIRMWARE_MAX_NUM_CHUNKS) {
return _report_status(OP_STATUS_ERR_LEN, output);
}
if (firmware_num_chunks > 0) {
diff --git a/src/da14531/da14531_handler.c b/src/da14531/da14531_handler.c
index 7c72262..99fff14 100644
--- a/src/da14531/da14531_handler.c
+++ b/src/da14531/da14531_handler.c
@@ -106,13 +106,13 @@ static void _ctrl_handler(const struct da14531_ctrl_frame* frame, struct RustByt
// util_log("da14531: bond db len %d", len);
uint16_t tmp_len;
uint8_t tmp[12 + sizeof(response) * 2];
- if (len != -1) {
+ if (len >= 0) {
tmp_len = da14531_protocol_format(
&tmp[0],
sizeof(tmp),
DA14531_PROTOCOL_PACKET_TYPE_CTRL_DATA,
&response[0],
- 1 + len);
+ 1 + (uint16_t)len);
} else {
tmp_len = da14531_protocol_format(
&tmp[0], sizeof(tmp), DA14531_PROTOCOL_PACKET_TYPE_CTRL_DATA, &response[0], 1);
@@ -129,7 +129,11 @@ static void _ctrl_handler(const struct da14531_ctrl_frame* frame, struct RustByt
ASSERT(false);
break;
}
- memory_set_ble_bond_db(&frame->cmd_data[0], frame->payload_length - 1);
+ if (!memory_set_ble_bond_db(&frame->cmd_data[0], frame->payload_length - 1)) {
+ util_log("da14531: set bond db failed");
+ ASSERT(false);
+ break;
+ }
#if FACTORYSETUP == 1
_bond_db_set = true;
#endif
diff --git a/src/memory/memory_shared.c b/src/memory/memory_shared.c
index 7545759..b8d27bb 100644
--- a/src/memory/memory_shared.c
+++ b/src/memory/memory_shared.c
@@ -163,9 +163,11 @@ int16_t memory_get_ble_bond_db(uint8_t* data)
chunk_shared_t chunk = {0};
memory_read_shared_bootdata(&chunk);
int16_t len = chunk.fields.ble_bond_db_len;
- if (len != -1) {
- memcpy(data, &chunk.fields.ble_bond_db[0], len);
+ if (len < 0 || len > MEMORY_BLE_BOND_DB_LEN) {
+ util_zero(&chunk, sizeof(chunk));
+ return -1;
}
+ memcpy(data, &chunk.fields.ble_bond_db[0], len);
util_zero(&chunk, sizeof(chunk));
return len;
@@ -173,8 +175,8 @@ int16_t memory_get_ble_bond_db(uint8_t* data)
bool memory_set_ble_bond_db(const uint8_t* data, int16_t data_len)
{
- ASSERT(data_len <= MEMORY_BLE_BOND_DB_LEN);
- if (data_len > MEMORY_BLE_BOND_DB_LEN) {
+ ASSERT(data_len >= 0 && data_len <= MEMORY_BLE_BOND_DB_LEN);
+ if (data_len < 0 || data_len > MEMORY_BLE_BOND_DB_LEN) {
return false;
}
chunk_shared_t chunk = {0};
diff --git a/src/memory/memory_shared.h b/src/memory/memory_shared.h
index 14dbd3f..b71f07f 100644
--- a/src/memory/memory_shared.h
+++ b/src/memory/memory_shared.h
@@ -144,7 +144,7 @@ void memory_get_ble_irk(uint8_t* data);
void memory_get_ble_identity_address(uint8_t* data);
// data_len can be at most MEMORY_BLE_BOND_DB_LEN
-bool memory_set_ble_bond_db(const uint8_t* data, int16_t data_len);
+USE_RESULT bool memory_set_ble_bond_db(const uint8_t* data, int16_t data_len);
typedef struct {
uint8_t allowed_firmware_hash[32];
diff --git a/src/rust/Cargo.lock b/src/rust/Cargo.lock
index d926ca1..6948aae 100644
--- a/src/rust/Cargo.lock
+++ b/src/rust/Cargo.lock
@@ -586,6 +586,7 @@ dependencies = [
"fiat-crypto",
"rustc_version 0.4.0",
"subtle",
+ "zeroize",
]
[[package]]
@@ -663,6 +664,7 @@ dependencies = [
"sha2",
"signature",
"subtle",
+ "zeroize",
]
[[package]]
@@ -1428,6 +1430,7 @@ checksum = "fb66477291e7e8d2b0ff1bcb900bf29489a9692816d79874bea351e7a8b6de96"
dependencies = [
"curve25519-dalek",
"rand_core",
+ "zeroize",
]
[[package]]
diff --git a/src/rust/bitbox02-noise/Cargo.toml b/src/rust/bitbox02-noise/Cargo.toml
index 13f9b83..e3f2626 100644
--- a/src/rust/bitbox02-noise/Cargo.toml
+++ b/src/rust/bitbox02-noise/Cargo.toml
@@ -21,4 +21,4 @@ features = ["use-sha2", "use-chacha20poly1305"]
[dependencies.x25519-dalek]
version = "2.0.0"
default-features = false
-features = ["static_secrets"]
+features = ["static_secrets", "zeroize"]
diff --git a/src/rust/bitbox02-rust/Cargo.toml b/src/rust/bitbox02-rust/Cargo.toml
index 56c48c4..d29be34 100644
--- a/src/rust/bitbox02-rust/Cargo.toml
+++ b/src/rust/bitbox02-rust/Cargo.toml
@@ -42,7 +42,7 @@ bip32-ed25519 = { git = "https://github.com/BitBoxSwiss/rust-bip32-ed25519", tag
blake2 = { version = "0.10.6", default-features = false, optional = true }
minicbor = { version = "0.24.0", default-features = false, features = ["alloc"], optional = true }
crc = { workspace = true, optional = true }
-ed25519-dalek = { version = "2.1.1", default-features = false, features = ["hazmat", "digest"], optional = true }
+ed25519-dalek = { version = "2.1.1", default-features = false, features = ["hazmat", "digest", "zeroize"], optional = true }
hmac = { workspace = true }
miniscript = { version = "13.0.0", default-features = false, features = [], optional = true }
diff --git a/src/rust/bitbox02-rust/src/backup.rs b/src/rust/bitbox02-rust/src/backup.rs
index 28533e4..d14d284 100644
--- a/src/rust/bitbox02-rust/src/backup.rs
+++ b/src/rust/bitbox02-rust/src/backup.rs
@@ -122,6 +122,17 @@ fn load_from_buffer(buf: &[u8]) -> Result<(Zeroizing<BackupData>, pb_backup::Bac
}
}
+fn load_from_buffer_for_dir(
+ buf: &[u8],
+ dir: &str,
+) -> Result<(Zeroizing<BackupData>, pb_backup::BackupMetaData), ()> {
+ let (backup_data, metadata) = load_from_buffer(buf)?;
+ if id(backup_data.get_seed()) != dir {
+ return Err(());
+ }
+ Ok((backup_data, metadata))
+}
+
/// Does a bitwise majority vote to recover the contents of potentially corrupted data. All three
/// buffers be of the same length.
fn bitwise_recovery(buf1: &[u8], buf2: &[u8], buf3: &[u8]) -> Result<Zeroizing<Vec<u8>>, ()> {
@@ -160,18 +171,17 @@ pub async fn load(
hal.sd().load_bin(&files[2], dir).await?,
];
for contents in file_contents.iter() {
- if let o @ Ok(_) = load_from_buffer(contents) {
+ if let o @ Ok(_) = load_from_buffer_for_dir(contents, dir) {
return o;
}
}
// If we arrived here, it means all three copies of the backup are corrupted (couldn't decode or
// failed the checksum verification). We try to recover with a bit-wise majority vote using the
// three copies of the backup. This only works if all three files have the same size.
- load_from_buffer(&bitwise_recovery(
- &file_contents[0],
- &file_contents[1],
- &file_contents[2],
- )?)
+ load_from_buffer_for_dir(
+ &bitwise_recovery(&file_contents[0], &file_contents[1], &file_contents[2])?,
+ dir,
+ )
}
pub async fn create(
@@ -344,6 +354,16 @@ mod tests {
&& contents[0].as_slice() == contents[2].as_slice()
);
+ let wrong_dir = "0000000000000000000000000000000000000000000000000000000000000000";
+ for (file, contents) in files.iter().zip(contents.iter()) {
+ mock_hal
+ .sd
+ .write_bin(file, wrong_dir, contents)
+ .await
+ .unwrap();
+ }
+ assert!(load(&mut mock_hal, wrong_dir).await.is_err());
+
// Recreating the backup removes the previous files.
assert!(
create(&mut mock_hal, seed, "new name", timestamp + 1, birthdate)
diff --git a/src/rust/bitbox02-rust/src/hww.rs b/src/rust/bitbox02-rust/src/hww.rs
index 6f0009b..10a6977 100644
--- a/src/rust/bitbox02-rust/src/hww.rs
+++ b/src/rust/bitbox02-rust/src/hww.rs
@@ -402,6 +402,11 @@ mod tests {
body: "Mon 2020-09-28".into(),
longtouch: false,
},
+ Screen::Confirm {
+ title: "".into(),
+ body: "Create backup?".into(),
+ longtouch: true,
+ },
Screen::Status {
title: "Backup created".into(),
success: true,
@@ -529,6 +534,11 @@ mod tests {
body: "Mon 2020-09-28".into(),
longtouch: false,
},
+ Screen::Confirm {
+ title: "".into(),
+ body: "Create backup?".into(),
+ longtouch: true,
+ },
Screen::Status {
title: "Backup created".into(),
success: true,
diff --git a/src/rust/bitbox02-rust/src/hww/api/backup.rs b/src/rust/bitbox02-rust/src/hww/api/backup.rs
index db4c43e..a61172a 100644
--- a/src/rust/bitbox02-rust/src/hww/api/backup.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/backup.rs
@@ -77,6 +77,15 @@ pub async fn create(
})
.await?;
+ hal.ui()
+ .confirm(&ConfirmParams {
+ title: "",
+ body: "Create backup?",
+ longtouch: true,
+ ..Default::default()
+ })
+ .await?;
+
// Wait for sd card
super::sdcard::process(
hal,
@@ -190,6 +199,11 @@ mod tests {
body: "Mon 2020-09-28".into(),
longtouch: false
},
+ Screen::Confirm {
+ title: "".into(),
+ body: "Create backup?".into(),
+ longtouch: true
+ },
Screen::Status {
title: "Backup created".into(),
success: true
@@ -245,6 +259,11 @@ mod tests {
body: "Mon 2020-09-28".into(),
longtouch: false
},
+ Screen::Confirm {
+ title: "".into(),
+ body: "Create backup?".into(),
+ longtouch: true
+ },
Screen::Status {
title: "Backup created".into(),
success: true
diff --git a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
index ba1f741..4731c0c 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
@@ -861,8 +861,8 @@ async fn _process(
// receiving the first output.
let mut empty_component = None;
- // Will contain the sum of all our output values (change or receive to self).
- let mut outputs_sum_ours: u64 = 0;
+ // Will contain the sum of all change output values.
+ let mut outputs_sum_change: u64 = 0;
// Will contain the sum of all outgoing output values (non-change outputs).
let mut outputs_sum_out: u64 = 0;
@@ -883,7 +883,11 @@ async fn _process(
// We don't allow regular outputs to have 0 value.
// OP_RETURN outputs however we require to have 0 value.
if output_type == pb::BtcOutputType::OpReturn {
- if tx_output.value != 0 {
+ if tx_output.value != 0
+ || tx_output.ours
+ || tx_output.silent_payment.is_some()
+ || tx_output.payment_request_index.is_some()
+ {
return Err(Error::InvalidInput);
}
} else if tx_output.value == 0 {
@@ -893,6 +897,12 @@ async fn _process(
// Get payload. If the output is marked ours, we compute the payload from the keystore,
// otherwise it is provided in tx_output.payload.
let payload: common::Payload = if tx_output.ours {
+ // Only external outputs can belong to a payment request. Receiving on silent payments
+ // is not supported yet.
+ if tx_output.payment_request_index.is_some() || tx_output.silent_payment.is_some() {
+ return Err(Error::InvalidInput);
+ }
+
// Compute the payload from the keystore.
let script_config_account =
if let Some(output_script_config_index) = tx_output.output_script_config_index {
@@ -969,15 +979,6 @@ async fn _process(
false
};
- // Only non-change outputs can belong to a payment request.
- if is_change && tx_output.payment_request_index.is_some() {
- return Err(Error::InvalidInput);
- }
-
- if is_change && tx_output.silent_payment.is_some() {
- return Err(Error::InvalidInput);
- }
-
if !is_change {
// Verify output if it is not a change output.
// Assemble address to display, get user confirmation.
@@ -1075,7 +1076,7 @@ async fn _process(
if is_change {
num_changes += 1;
- outputs_sum_ours = outputs_sum_ours
+ outputs_sum_change = outputs_sum_change
.checked_add(tx_output.value)
.ok_or(Error::InvalidInput)?;
} else {
@@ -1137,7 +1138,7 @@ async fn _process(
// Total out, including fee.
let total_out: u64 = inputs_sum_pass1
- .checked_sub(outputs_sum_ours)
+ .checked_sub(outputs_sum_change)
.ok_or(Error::InvalidInput)?;
let fee: u64 = total_out
.checked_sub(outputs_sum_out)
@@ -2648,6 +2649,34 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_silent_payment_rejects_ours_output() {
+ let transaction =
+ alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
+
+ {
+ let mut tx = transaction.borrow_mut();
+ let silent_payment_output_index = 5;
+ assert!(tx.outputs[silent_payment_output_index].ours);
+ // A receive-branch self-transfer is not change, but it is still marked as ours and
+ // must not carry a silent payment address.
+ tx.outputs[silent_payment_output_index].keypath[3] = 0;
+ tx.outputs[silent_payment_output_index].silent_payment =
+ Some(pb::btc_sign_output_request::SilentPayment {
+ address: "sp1qqgste7k9hx0qftg6qmwlkqtwuy6cycyavzmzj85c6qdfhjdpdjtdgqjuexzk6murw56suy3e0rd2cgqvycxttddwsvgxe2usfpxumr70xc9pkqwv".into(),
+ });
+ }
+
+ mock_host_responder(transaction.clone());
+ mock_unlocked();
+ let init_request = transaction.borrow().init_request();
+
+ assert_eq!(
+ process(&mut TestingHal::new(), &init_request).await,
+ Err(Error::InvalidInput)
+ );
+ }
+
// Test an output that is sending to the same account, but is not a change output by keypath.
#[async_test::test]
async fn test_self_send_non_change_output_same_account() {
@@ -3791,6 +3820,50 @@ mod tests {
);
}
+ #[async_test::test]
+ pub async fn test_payment_request_rejects_ours_output() {
+ let transaction =
+ alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
+
+ {
+ let mut tx = transaction.borrow_mut();
+ let payment_request_output_index = 5;
+ assert!(tx.outputs[payment_request_output_index].ours);
+ let output_value = tx.outputs[payment_request_output_index].value;
+ let mut payment_request = pb::BtcPaymentRequestRequest {
+ recipient_name: "Test Merchant".into(),
+ memos: vec![Memo {
+ memo: Some(memo::Memo::TextMemo(memo::TextMemo {
+ note: "Test memo".into(),
+ })),
+ }],
+ nonce: vec![],
+ total_amount: output_value,
+ signature: vec![],
+ };
+ payment_request::tst_sign_payment_request_btc(
+ tx.coin,
+ &mut payment_request,
+ output_value,
+ "bc1qnu4x8dlrx6dety47gehf4uhk5tj3q7yhywgry6",
+ );
+ tx.payment_request = Some(payment_request);
+ // A receive-branch self-transfer is not change, but it is still marked as ours and
+ // must not belong to a payment request.
+ tx.outputs[payment_request_output_index].keypath[3] = 0;
+ tx.outputs[payment_request_output_index].payment_request_index = Some(0);
+ }
+
+ mock_host_responder(transaction.clone());
+ mock_unlocked();
+ let init_request = transaction.borrow().init_request();
+
+ assert_eq!(
+ process(&mut TestingHal::new(), &init_request).await,
+ Err(Error::InvalidInput)
+ );
+ }
+
#[cfg(feature = "app-ethereum")]
#[test]
pub fn test_validate_swap_source_account() {
@@ -4098,4 +4171,80 @@ mod tests {
Err(Error::InvalidInput)
);
}
+
+ #[async_test::test]
+ async fn test_op_return_rejects_ours_output() {
+ let transaction =
+ alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
+
+ {
+ let mut tx = transaction.borrow_mut();
+ let output_index = 5;
+ assert!(tx.outputs[output_index].ours);
+ tx.outputs[output_index].r#type = pb::BtcOutputType::OpReturn as _;
+ tx.outputs[output_index].value = 0;
+ tx.outputs[output_index].payload = b"hello world".to_vec();
+ tx.outputs[output_index].keypath[3] = 0;
+ }
+
+ mock_host_responder(transaction.clone());
+ mock_unlocked();
+ let init_request = transaction.borrow().init_request();
+
+ assert_eq!(
+ process(&mut TestingHal::new(), &init_request).await,
+ Err(Error::InvalidInput)
+ );
+ }
+
+ #[async_test::test]
+ async fn test_op_return_rejects_silent_payment_output() {
+ let transaction =
+ alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
+
+ {
+ let mut tx = transaction.borrow_mut();
+ tx.outputs[0].r#type = pb::BtcOutputType::OpReturn as _;
+ tx.outputs[0].value = 0;
+ tx.outputs[0].payload = b"hello world".to_vec();
+ tx.outputs[0].silent_payment = Some(pb::btc_sign_output_request::SilentPayment {
+ address: "sp1qqgste7k9hx0qftg6qmwlkqtwuy6cycyavzmzj85c6qdfhjdpdjtdgqjuexzk6murw56suy3e0rd2cgqvycxttddwsvgxe2usfpxumr70xc9pkqwv".into(),
+ });
+ }
+
+ mock_host_responder(transaction.clone());
+ mock_unlocked();
+ let init_request = transaction.borrow().init_request();
+
+ assert_eq!(
+ process(&mut TestingHal::new(), &init_request).await,
+ Err(Error::InvalidInput)
+ );
+ }
+
+ #[async_test::test]
+ async fn test_op_return_rejects_payment_request_output() {
+ let transaction =
+ alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
+
+ {
+ let mut tx = transaction.borrow_mut();
+ tx.outputs.push(pb::BtcSignOutputRequest {
+ r#type: pb::BtcOutputType::OpReturn as _,
+ value: 0,
+ payload: b"hello world".to_vec(),
+ payment_request_index: Some(0),
+ ..Default::default()
+ });
+ }
+
+ mock_host_responder(transaction.clone());
+ mock_unlocked();
+ let init_request = transaction.borrow().init_request();
+
+ assert_eq!(
+ process(&mut TestingHal::new(), &init_request).await,
+ Err(Error::InvalidInput)
+ );
+ }
}
diff --git a/src/rust/bitbox02-rust/src/hww/api/cardano/sign_transaction.rs b/src/rust/bitbox02-rust/src/hww/api/cardano/sign_transaction.rs
index 80195f1..e099f7b 100644
--- a/src/rust/bitbox02-rust/src/hww/api/cardano/sign_transaction.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/cardano/sign_transaction.rs
@@ -122,6 +122,12 @@ fn validate_asset_groups(
Ok(())
}
+fn commits_to_network(request: &pb::CardanoSignTransactionRequest) -> bool {
+ // The tx body network_id field is not encoded. Outputs and withdrawals still bind the selected
+ // network through their address bytes.
+ !request.outputs.is_empty() || !request.withdrawals.is_empty()
+}
+
async fn _process(
hal: &mut impl crate::hal::Hal,
request: &pb::CardanoSignTransactionRequest,
@@ -181,6 +187,17 @@ async fn _process(
}
}
}
+ if !commits_to_network(request) {
+ hal.ui()
+ .confirm(&ConfirmParams {
+ title: "Warning",
+ body: "This transaction does not encode a Cardano network and may be valid on another Cardano network.",
+ accept_is_nextarrow: true,
+ scrollable: true,
+ ..Default::default()
+ })
+ .await?;
+ }
certificates::verify(
hal,
params,
@@ -244,7 +261,7 @@ async fn _process(
hal.ui()
.verify_recipient(&displayed_address, &formatted_value)
.await?;
- total += output.value;
+ total = total.checked_add(output.value).ok_or(Error::InvalidInput)?;
for asset_group in output.asset_groups.iter() {
for token in asset_group.tokens.iter() {
@@ -277,10 +294,11 @@ async fn _process(
})
.await?;
} else {
+ let total_with_fee = total.checked_add(request.fee).ok_or(Error::InvalidInput)?;
let fee_percentage: f64 = 100. * (request.fee as f64) / (total as f64);
transaction::verify_total_fee_maybe_warn(
hal,
- &format_value(params, total + request.fee),
+ &format_value(params, total_with_fee),
&format_value(params, request.fee),
Some(fee_percentage),
)
@@ -585,6 +603,64 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_sign_stake_registration_no_network_commitment_warning() {
+ let tx = pb::CardanoSignTransactionRequest {
+ network: CardanoNetwork::CardanoMainnet as _,
+ inputs: vec![pb::cardano_sign_transaction_request::Input {
+ keypath: vec![1852 + HARDENED, 1815 + HARDENED, HARDENED, 0, 0],
+ prev_out_hash: b"\x64\xc3\x9d\x60\xf9\xd6\xb4\xf8\x83\xd0\x5a\xe3\x58\x5d\x06\x21\xd0\xfe\xbc\x06\xad\x0e\xa3\x40\x3b\xdc\x00\xbc\x23\x67\x16\x15".to_vec(),
+ prev_out_index: 1,
+ }],
+ fee: 191681,
+ ttl: 41539125,
+ certificates: vec![Certificate {
+ cert: Some(Cert::StakeRegistration(pb::Keypath {
+ keypath: vec![1852 + HARDENED, 1815 + HARDENED, HARDENED, 2, 0],
+ })),
+ }],
+ ..Default::default()
+ };
+
+ mock_unlocked();
+
+ let mut mock_hal = TestingHal::new();
+ let result = process(&mut mock_hal, &tx).await.unwrap();
+ let Response::SignTransaction(response) = result else {
+ panic!("unexpected response");
+ };
+ assert_eq!(response.shelley_witnesses.len(), 2);
+ assert_eq!(
+ mock_hal.ui.screens,
+ vec![
+ Screen::Confirm {
+ title: "Cardano".into(),
+ body: "Can be mined until\nslot 326325 in\nepoch 293".into(),
+ longtouch: false
+ },
+ Screen::Confirm {
+ title: "Warning".into(),
+ body: "This transaction does not encode a Cardano network and may be valid on another Cardano network.".into(),
+ longtouch: false
+ },
+ Screen::Confirm {
+ title: "Cardano".into(),
+ body: "Register staking key for account #1?".into(),
+ longtouch: false
+ },
+ Screen::Confirm {
+ title: "Cardano".into(),
+ body: "Fee\n0.191681 ADA".into(),
+ longtouch: true
+ },
+ Screen::Status {
+ title: "Transaction\nconfirmed".into(),
+ success: true
+ }
+ ]
+ );
+ }
+
#[async_test::test]
async fn test_sign_stake_deregistration() {
let tx = pb::CardanoSignTransactionRequest {
@@ -844,6 +920,58 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_sign_withdrawal_no_outputs_commits_to_network() {
+ let tx = pb::CardanoSignTransactionRequest {
+ network: CardanoNetwork::CardanoMainnet as _,
+ inputs: vec![pb::cardano_sign_transaction_request::Input {
+ keypath: vec![1852 + HARDENED, 1815 + HARDENED, HARDENED, 0, 0],
+ prev_out_hash: b"\xb7\xb2\x33\x3e\x72\xf2\x67\x0a\xb8\x20\x51\xf4\x26\xcc\x84\x00\x04\x31\x97\x5a\x34\xe7\x1d\x5e\xdf\x70\xea\x6c\x0d\xdc\x9b\xf8".to_vec(),
+ prev_out_index: 0,
+ }],
+ fee: 175157,
+ ttl: 41788708,
+ withdrawals: vec![pb::cardano_sign_transaction_request::Withdrawal {
+ keypath: vec![1852 + HARDENED, 1815 + HARDENED, HARDENED, 2, 0],
+ value: 1234567,
+ }],
+ ..Default::default()
+ };
+
+ mock_unlocked();
+
+ let mut mock_hal = TestingHal::new();
+ let result = process(&mut mock_hal, &tx).await.unwrap();
+ let Response::SignTransaction(response) = result else {
+ panic!("unexpected response");
+ };
+ assert_eq!(response.shelley_witnesses.len(), 2);
+ assert_eq!(
+ mock_hal.ui.screens,
+ vec![
+ Screen::Confirm {
+ title: "Cardano".into(),
+ body: "Can be mined until\nslot 143908 in\nepoch 294".into(),
+ longtouch: false
+ },
+ Screen::Confirm {
+ title: "Cardano".into(),
+ body: "Withdraw 1.234567 ADA in staking rewards for account #1?".into(),
+ longtouch: false
+ },
+ Screen::Confirm {
+ title: "Cardano".into(),
+ body: "Fee\n0.175157 ADA".into(),
+ longtouch: true
+ },
+ Screen::Status {
+ title: "Transaction\nconfirmed".into(),
+ success: true
+ }
+ ]
+ );
+ }
+
/// Test that ttl=0 is not included in the transaction if allow_ttl_zero is false. Up to v9.8.0, ttl was not included if it was zero.
#[async_test::test]
async fn test_sign_tx_no_ttl() {
@@ -1232,6 +1360,67 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_reject_external_output_total_overflow() {
+ let tx = pb::CardanoSignTransactionRequest {
+ network: CardanoNetwork::CardanoMainnet as _,
+ inputs: vec![pb::cardano_sign_transaction_request::Input {
+ keypath: vec![1852 + HARDENED, 1815 + HARDENED, HARDENED, 0, 0],
+ prev_out_hash: b"\x59\x86\x4e\xe7\x3c\xa5\xd9\x10\x98\xa3\x2b\x3c\xe9\x81\x1b\xac\x19\x96\xdc\xba\xef\xa6\xb6\x24\x7d\xca\xaf\xb5\x77\x9c\x25\x38".to_vec(),
+ prev_out_index: 0,
+ }],
+ outputs: vec![
+ pb::cardano_sign_transaction_request::Output {
+ encoded_address: "addr1q9qfllpxg2vu4lq6rnpel4pvpp5xnv3kvvgtxk6k6wp4ff89xrhu8jnu3p33vnctc9eklee5dtykzyag5penc6dcmakqsqqgpt".into(),
+ value: u64::MAX,
+ script_config: None,
+ asset_groups: vec![],
+ },
+ pb::cardano_sign_transaction_request::Output {
+ encoded_address: "addr1q9qfllpxg2vu4lq6rnpel4pvpp5xnv3kvvgtxk6k6wp4ff89xrhu8jnu3p33vnctc9eklee5dtykzyag5penc6dcmakqsqqgpt".into(),
+ value: 1,
+ script_config: None,
+ asset_groups: vec![],
+ },
+ ],
+ ..Default::default()
+ };
+
+ mock_unlocked();
+ let mut mock_hal = TestingHal::new();
+ assert!(matches!(
+ process(&mut mock_hal, &tx).await,
+ Err(Error::InvalidInput)
+ ));
+ }
+
+ #[async_test::test]
+ async fn test_reject_total_with_fee_overflow() {
+ let tx = pb::CardanoSignTransactionRequest {
+ network: CardanoNetwork::CardanoMainnet as _,
+ inputs: vec![pb::cardano_sign_transaction_request::Input {
+ keypath: vec![1852 + HARDENED, 1815 + HARDENED, HARDENED, 0, 0],
+ prev_out_hash: b"\x59\x86\x4e\xe7\x3c\xa5\xd9\x10\x98\xa3\x2b\x3c\xe9\x81\x1b\xac\x19\x96\xdc\xba\xef\xa6\xb6\x24\x7d\xca\xaf\xb5\x77\x9c\x25\x38".to_vec(),
+ prev_out_index: 0,
+ }],
+ outputs: vec![pb::cardano_sign_transaction_request::Output {
+ encoded_address: "addr1q9qfllpxg2vu4lq6rnpel4pvpp5xnv3kvvgtxk6k6wp4ff89xrhu8jnu3p33vnctc9eklee5dtykzyag5penc6dcmakqsqqgpt".into(),
+ value: u64::MAX,
+ script_config: None,
+ asset_groups: vec![],
+ }],
+ fee: 1,
+ ..Default::default()
+ };
+
+ mock_unlocked();
+ let mut mock_hal = TestingHal::new();
+ assert!(matches!(
+ process(&mut mock_hal, &tx).await,
+ Err(Error::InvalidInput)
+ ));
+ }
+
#[async_test::test]
async fn test_sign_tx_tag_cbor_sets() {
let tx = pb::CardanoSignTransactionRequest {
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
index b25c58a..28232a8 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign.rs
@@ -25,6 +25,11 @@ use num_bigint::BigUint;
// 1 ETH = 1e18 wei.
const WEI_DECIMALS: usize = 18;
+
+// Streaming data size limits
+const MAX_STREAMING_DATA_LENGTH: u32 = 1024 * 1024;
+const MAX_NONSTREAMING_DATA_LENGTH: usize = 6144;
+
pub enum Transaction<'a> {
Legacy(&'a pb::EthSignRequest),
Eip1559(&'a pb::EthSignEip1559Request),
@@ -507,13 +512,13 @@ pub async fn _process(
}
// Size limits.
- const MAX_STREAMING_DATA_LENGTH: u32 = 1024 * 1024;
- const MAX_NONSTREAMING_DATA_LENGTH: usize = 6144;
if request.nonce().len() > 16
|| request.gas_limit().len() > 16
|| request.value().len() > 32
|| request.data().len() > MAX_NONSTREAMING_DATA_LENGTH
|| request.data_length() > MAX_STREAMING_DATA_LENGTH
+ || (request.data_length() > 0
+ && request.data_length() <= MAX_NONSTREAMING_DATA_LENGTH as u32)
{
return Err(Error::InvalidInput);
}
@@ -1866,7 +1871,7 @@ mod tests {
}
#[async_test::test]
- pub async fn test_streaming_equivalence_legacy() {
+ pub async fn test_process_streaming_below_threshold_rejected_legacy() {
const KEYPATH: &[u32] = &[44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0];
let test_data: Vec<u8> = (0..4000u32).map(|i| (i % 256) as u8).collect();
@@ -1915,74 +1920,7 @@ mod tests {
clear_chunk_responder();
assert!(nonstreaming_result.is_ok());
- assert!(streaming_result.is_ok());
- match (&nonstreaming_result, &streaming_result) {
- (Ok(Response::Sign(trad)), Ok(Response::Sign(stream))) => {
- assert_eq!(trad.signature, stream.signature);
- }
- _ => panic!("Expected Sign responses from both modes"),
- }
- }
-
- #[async_test::test]
- pub async fn test_streaming_equivalence_eip1559() {
- const KEYPATH: &[u32] = &[44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0];
- let test_data: Vec<u8> = (0..4000u32).map(|i| (i % 256) as u8).collect();
-
- mock_unlocked();
- let mut mock_hal_nonstreaming = TestingHal::new();
- let nonstreaming_result = process(
- &mut mock_hal_nonstreaming,
- &Transaction::Eip1559(&pb::EthSignEip1559Request {
- keypath: KEYPATH.to_vec(),
- nonce: hex!("01").to_vec(),
- max_priority_fee_per_gas: hex!("3b9aca00").to_vec(),
- max_fee_per_gas: hex!("04a817c800").to_vec(),
- gas_limit: hex!("0f4240").to_vec(),
- recipient: hex!("112233445566778899aabbccddeeff0011223344").to_vec(),
- value: b"".to_vec(),
- data: test_data.clone(),
- host_nonce_commitment: None,
- chain_id: 1,
- address_case: pb::EthAddressCase::Mixed as _,
- data_length: 0,
- payment_request: None,
- }),
- )
- .await;
-
- setup_chunk_responder(test_data.clone());
- mock_unlocked();
- let mut mock_hal_streaming = TestingHal::new();
- let streaming_result = process(
- &mut mock_hal_streaming,
- &Transaction::Eip1559(&pb::EthSignEip1559Request {
- keypath: KEYPATH.to_vec(),
- nonce: hex!("01").to_vec(),
- max_priority_fee_per_gas: hex!("3b9aca00").to_vec(),
- max_fee_per_gas: hex!("04a817c800").to_vec(),
- gas_limit: hex!("0f4240").to_vec(),
- recipient: hex!("112233445566778899aabbccddeeff0011223344").to_vec(),
- value: b"".to_vec(),
- data: vec![],
- host_nonce_commitment: None,
- chain_id: 1,
- address_case: pb::EthAddressCase::Mixed as _,
- data_length: 4000,
- payment_request: None,
- }),
- )
- .await;
- clear_chunk_responder();
-
- assert!(nonstreaming_result.is_ok());
- assert!(streaming_result.is_ok());
- match (&nonstreaming_result, &streaming_result) {
- (Ok(Response::Sign(trad)), Ok(Response::Sign(stream))) => {
- assert_eq!(trad.signature, stream.signature);
- }
- _ => panic!("Expected Sign responses from both modes"),
- }
+ assert_eq!(streaming_result, Err(Error::InvalidInput));
}
#[async_test::test]
@@ -2132,9 +2070,11 @@ mod tests {
}
#[async_test::test]
- pub async fn test_streaming_1_byte_legacy() {
+ pub async fn test_process_streaming_at_threshold_rejected_legacy() {
const KEYPATH: &[u32] = &[44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0];
- let test_data: Vec<u8> = vec![0x42];
+ let test_data: Vec<u8> = (0..MAX_NONSTREAMING_DATA_LENGTH as u32)
+ .map(|i| (i % 256) as u8)
+ .collect();
setup_chunk_responder(test_data);
mock_unlocked();
@@ -2153,17 +2093,12 @@ mod tests {
host_nonce_commitment: None,
chain_id: 1,
address_case: pb::EthAddressCase::Mixed as _,
- data_length: 1,
+ data_length: MAX_NONSTREAMING_DATA_LENGTH as u32,
}),
)
.await;
clear_chunk_responder();
- match result {
- Ok(Response::Sign(ref sig)) => {
- assert_eq!(sig.signature.len(), 65);
- }
- other => panic!("expected Ok(Sign), got {:?}", other),
- }
+ assert_eq!(result, Err(Error::InvalidInput));
}
#[async_test::test]
diff --git a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
index 286f5d5..c6fe74f 100644
--- a/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/ethereum/sign_typed_msg.rs
@@ -35,10 +35,74 @@ const DOMAIN_TYPE_NAME: &str = "EIP712Domain";
const MAX_TYPED_MSG_STREAMING_DATA_LENGTH: u32 = 1024 * 1024;
+struct CachedChainId {
+ index: u32,
+ req: pb::EthTypedMessageValueRequest,
+}
+
+fn cached_chain_id_value(
+ cached_chain_id: Option<&CachedChainId>,
+ root_object: RootObject,
+ path: &[u32],
+) -> Option<pb::EthTypedMessageValueRequest> {
+ let cached_chain_id = cached_chain_id?;
+ if root_object != RootObject::Domain {
+ return None;
+ }
+ match path {
+ [index] if *index == cached_chain_id.index => Some(cached_chain_id.req.clone()),
+ _ => None,
+ }
+}
+
fn get_type<'a>(types: &'a [StructType], name: &str) -> Option<&'a StructType> {
types.iter().find(|t| t.name == name)
}
+fn is_identifier_start(byte: u8) -> bool {
+ matches!(byte, b'A'..=b'Z' | b'a'..=b'z' | b'_' | b'$')
+}
+
+fn is_identifier_byte(byte: u8) -> bool {
+ is_identifier_start(byte) || byte.is_ascii_digit()
+}
+
+fn validate_identifier(name: &str) -> Result<(), Error> {
+ let mut bytes = name.bytes();
+ match bytes.next() {
+ Some(byte) if is_identifier_start(byte) => {}
+ _ => return Err(Error::InvalidInput),
+ }
+ if bytes.all(is_identifier_byte) {
+ Ok(())
+ } else {
+ Err(Error::InvalidInput)
+ }
+}
+
+fn validate_member_type_identifiers(typ: &MemberType) -> Result<(), Error> {
+ match DataType::try_from(typ.r#type)? {
+ DataType::Unknown => Err(Error::InvalidInput),
+ DataType::Array => {
+ validate_member_type_identifiers(typ.array_type.as_ref().ok_or(Error::InvalidInput)?)
+ }
+ DataType::Struct => validate_identifier(&typ.struct_name),
+ _ => Ok(()),
+ }
+}
+
+fn validate_typed_msg_schema(types: &[StructType], primary_type: &str) -> Result<(), Error> {
+ validate_identifier(primary_type)?;
+ for typ in types {
+ validate_identifier(&typ.name)?;
+ for member in &typ.members {
+ validate_identifier(&member.name)?;
+ validate_member_type_identifiers(member.r#type.as_ref().ok_or(Error::InvalidInput)?)?;
+ }
+ }
+ Ok(())
+}
+
fn get_transitive_types<'a>(types: &'a [StructType], name: &'a str) -> Result<Vec<&'a str>, Error> {
fn rec<'a>(
types: &'a [StructType],
@@ -291,44 +355,35 @@ fn format_display_line_body(
)
}
-#[allow(clippy::too_many_arguments)]
+struct HashContext<'a> {
+ types: &'a [StructType],
+ root_object: RootObject,
+ path: &'a [u32],
+ formatted_path: &'a [String],
+ title_suffix: Option<String>,
+ cached_chain_id: Option<&'a CachedChainId>,
+}
+
async fn encode_member<U: sha3::digest::Update>(
hal: &mut impl crate::hal::Hal,
hasher: &mut U,
- types: &[StructType],
member_type: &MemberType,
- root_object: RootObject,
- path: &[u32],
- formatted_path: &[String],
- title_suffix: Option<String>,
+ context: &HashContext<'_>,
) -> Result<(), Error> {
if member_type.r#type == DataType::Struct as i32 {
- let value_encoded = Box::pin(hash_struct(
- hal,
- types,
- root_object,
- &member_type.struct_name,
- path,
- formatted_path,
- title_suffix,
- ))
- .await?;
+ let value_encoded = Box::pin(hash_struct(hal, &member_type.struct_name, context)).await?;
hasher.update(&value_encoded);
} else if member_type.r#type == DataType::Array as i32 {
- let encoded_value = Box::pin(hash_array(
- hal,
- types,
- member_type,
- root_object,
- path,
- formatted_path,
- title_suffix,
- ))
- .await?;
+ let encoded_value = Box::pin(hash_array(hal, member_type, context)).await?;
hasher.update(&encoded_value);
} else {
- let req = get_value_from_host(root_object, path).await?;
- let display_path = formatted_path.join(".");
+ let req =
+ match cached_chain_id_value(context.cached_chain_id, context.root_object, context.path)
+ {
+ Some(req) => req,
+ None => get_value_from_host(context.root_object, context.path).await?,
+ };
+ let display_path = context.formatted_path.join(".");
let data_type = DataType::try_from(member_type.r#type)?;
let mut display_size = 0usize;
@@ -386,8 +441,8 @@ async fn encode_member<U: sha3::digest::Update>(
.confirm(&ConfirmParams {
title: &format!(
"{}{}",
- confirm_title(root_object),
- title_suffix.as_deref().unwrap_or("")
+ confirm_title(context.root_object),
+ context.title_suffix.as_deref().unwrap_or("")
),
body: &body,
scrollable: true,
@@ -403,17 +458,13 @@ async fn encode_member<U: sha3::digest::Update>(
async fn hash_array(
hal: &mut impl crate::hal::Hal,
- types: &[StructType],
member_type: &MemberType,
- root_object: RootObject,
- path: &[u32],
- formatted_path: &[String],
- title_suffix: Option<String>,
+ context: &HashContext<'_>,
) -> Result<Vec<u8>, Error> {
let array_size = if member_type.size > 0 {
member_type.size
} else {
- let req = get_value_from_host(root_object, path).await?;
+ let req = get_value_from_host(context.root_object, context.path).await?;
if req.data_length > 0 {
return Err(Error::InvalidInput);
}
@@ -426,12 +477,12 @@ async fn hash_array(
.confirm(&ConfirmParams {
title: &format!(
"{}{}",
- confirm_title(root_object),
- title_suffix.as_deref().unwrap_or("")
+ confirm_title(context.root_object),
+ context.title_suffix.as_deref().unwrap_or("")
),
body: &format!(
"{}: {}",
- formatted_path.join("."),
+ context.formatted_path.join("."),
if array_size == 0 {
"(empty list)".into()
} else {
@@ -445,8 +496,8 @@ async fn hash_array(
.await?;
let mut hasher = sha3::Keccak256::new();
- let mut child_path = path.to_vec();
- let mut child_formatted_path = formatted_path.to_vec();
+ let mut child_path = context.path.to_vec();
+ let mut child_formatted_path = context.formatted_path.to_vec();
child_path.push(0);
let member_name = child_formatted_path.last().unwrap().clone();
for index in 0..array_size {
@@ -454,37 +505,31 @@ async fn hash_array(
*child_formatted_path.last_mut().unwrap() =
format!("{}[{}/{}]", member_name, index + 1, array_size);
- encode_member(
- hal,
- &mut hasher,
- types,
- array_type,
- root_object,
- &child_path,
- &child_formatted_path,
- title_suffix.clone(),
- )
- .await?;
+ let child_context = HashContext {
+ types: context.types,
+ root_object: context.root_object,
+ path: &child_path,
+ formatted_path: &child_formatted_path,
+ title_suffix: context.title_suffix.clone(),
+ cached_chain_id: context.cached_chain_id,
+ };
+ encode_member(hal, &mut hasher, array_type, &child_context).await?;
}
Ok(hasher.finalize().to_vec())
}
async fn hash_struct(
hal: &mut impl crate::hal::Hal,
- types: &[StructType],
- root_object: RootObject,
struct_name: &str,
- path: &[u32],
- formatted_path: &[String],
- title_suffix: Option<String>,
+ context: &HashContext<'_>,
) -> Result<Vec<u8>, Error> {
let mut hasher = sha3::Keccak256::new();
- hasher.update(&type_hash(types, struct_name)?);
+ hasher.update(&type_hash(context.types, struct_name)?);
- let typ = get_type(types, struct_name).ok_or(Error::InvalidInput)?;
- let mut child_path = path.to_vec();
+ let typ = get_type(context.types, struct_name).ok_or(Error::InvalidInput)?;
+ let mut child_path = context.path.to_vec();
child_path.push(0);
- let mut child_formatted_path = formatted_path.to_vec();
+ let mut child_formatted_path = context.formatted_path.to_vec();
child_formatted_path.push("".into());
for (index, member) in typ.members.iter().enumerate() {
*child_path.last_mut().unwrap() = index as u32;
@@ -493,21 +538,19 @@ async fn hash_struct(
.unwrap()
.clone_from(&member.name);
let member_type = member.r#type.as_ref().ok_or(Error::InvalidInput)?;
- encode_member(
- hal,
- &mut hasher,
- types,
- member_type,
- root_object,
- &child_path,
- &child_formatted_path,
- if title_suffix.is_some() {
- title_suffix.clone()
+ let child_context = HashContext {
+ types: context.types,
+ root_object: context.root_object,
+ path: &child_path,
+ formatted_path: &child_formatted_path,
+ title_suffix: if context.title_suffix.is_some() {
+ context.title_suffix.clone()
} else {
Some(format!(" ({}/{})", index + 1, typ.members.len()))
},
- )
- .await?;
+ cached_chain_id: context.cached_chain_id,
+ };
+ encode_member(hal, &mut hasher, member_type, &child_context).await?;
}
Ok(hasher.finalize().to_vec())
@@ -517,7 +560,9 @@ async fn hash_struct(
/// it matches chain ID provided in the request. In theory, the chain ID can be known to the wallet
/// app without including it in the domain to be signed, which is why it is provided directly in the
/// request regardless of whether it is present in the domain.
-async fn validate_chain_id(request: &pb::EthSignTypedMessageRequest) -> Result<(), Error> {
+async fn validate_chain_id(
+ request: &pb::EthSignTypedMessageRequest,
+) -> Result<Option<CachedChainId>, Error> {
let domain_type = get_type(&request.types, DOMAIN_TYPE_NAME).ok_or(Error::InvalidInput)?;
let chain_id_index = match domain_type
.members
@@ -526,10 +571,19 @@ async fn validate_chain_id(request: &pb::EthSignTypedMessageRequest) -> Result<(
{
Some(i) => i,
None => {
- // Chain ID is not part of the domain - skip validation
- return Ok(());
+ return Ok(None);
}
};
+ let chain_id_type = domain_type.members[chain_id_index]
+ .r#type
+ .as_ref()
+ .ok_or(Error::InvalidInput)?;
+ if DataType::try_from(chain_id_type.r#type)? != DataType::Uint
+ || chain_id_type.size == 0
+ || chain_id_type.size > 32
+ {
+ return Err(Error::InvalidInput);
+ }
let req = get_value_from_host(RootObject::Domain, &[chain_id_index as u32]).await?;
if req.data_length > 0 {
return Err(Error::InvalidInput);
@@ -540,41 +594,43 @@ async fn validate_chain_id(request: &pb::EthSignTypedMessageRequest) -> Result<(
if chain_id != request.chain_id {
return Err(Error::InvalidInput);
}
- Ok(())
+ Ok(Some(CachedChainId {
+ index: chain_id_index as u32,
+ req,
+ }))
}
async fn eip712_sighash(
hal: &mut impl crate::hal::Hal,
types: &[StructType],
primary_type: &str,
+ cached_chain_id: Option<&CachedChainId>,
) -> Result<[u8; 32], Error> {
let mut hasher = sha3::Keccak256::new();
hasher.update([0x19u8, 0x01]);
- let domain_separator = hash_struct(
- hal,
+ let domain_context = HashContext {
types,
- RootObject::Domain,
- DOMAIN_TYPE_NAME,
- &[],
- &[],
- None,
- )
- .await?;
+ root_object: RootObject::Domain,
+ path: &[],
+ formatted_path: &[],
+ title_suffix: None,
+ cached_chain_id,
+ };
+ let domain_separator = hash_struct(hal, DOMAIN_TYPE_NAME, &domain_context).await?;
hasher.update(&domain_separator);
// If primaryType is the domain type, skip the message hashing. This does not seem to conform to
// the spec, but eth-sig-util implements it like that:
// https://github.com/MetaMask/eth-sig-util/pull/51#issuecomment-1135089739
if primary_type != DOMAIN_TYPE_NAME {
- let message_struct_hash = hash_struct(
- hal,
+ let message_context = HashContext {
types,
- RootObject::Message,
- primary_type,
- &[],
- &[],
- None,
- )
- .await?;
+ root_object: RootObject::Message,
+ path: &[],
+ formatted_path: &[],
+ title_suffix: None,
+ cached_chain_id: None,
+ };
+ let message_struct_hash = hash_struct(hal, primary_type, &message_context).await?;
hasher.update(&message_struct_hash);
}
Ok(hasher.finalize().into())
@@ -590,7 +646,20 @@ pub async fn process(
hal: &mut impl crate::hal::Hal,
request: &pb::EthSignTypedMessageRequest,
) -> Result<Response, Error> {
- validate_chain_id(request).await?;
+ validate_typed_msg_schema(&request.types, &request.primary_type)?;
+
+ let cached_chain_id = validate_chain_id(request).await?;
+ if cached_chain_id.is_none() {
+ hal.ui()
+ .confirm(&ConfirmParams {
+ title: "Warning",
+ body: "Typed data has no chain ID. Message is valid for every chain.",
+ scrollable: true,
+ accept_is_nextarrow: true,
+ ..Default::default()
+ })
+ .await?;
+ }
// Base component on the screen stack during signing, which is shown while the device is waiting
// for the next signing api call. Without this, the 'See the BitBoxApp' waiting screen would
@@ -612,7 +681,13 @@ pub async fn process(
)
.await?;
- let sighash: [u8; 32] = eip712_sighash(hal, &request.types, &request.primary_type).await?;
+ let sighash: [u8; 32] = eip712_sighash(
+ hal,
+ &request.types,
+ &request.primary_type,
+ cached_chain_id.as_ref(),
+ )
+ .await?;
hal.ui()
.confirm(&ConfirmParams {
@@ -704,6 +779,58 @@ mod tests {
}
}
+ fn chain_id_value(chain_id: Option<u64>) -> Vec<u8> {
+ chain_id
+ .map(|chain_id| BigUint::from(chain_id).to_bytes_be())
+ .unwrap_or_default()
+ }
+
+ fn setup_chain_id_responder_panicking_on_second_request(
+ chain_id: Option<u64>,
+ ) -> alloc::rc::Rc<core::cell::Cell<usize>> {
+ let request_count = alloc::rc::Rc::new(core::cell::Cell::new(0usize));
+ let request_count_clone = request_count.clone();
+ *crate::hww::MOCK_NEXT_REQUEST.0.borrow_mut() =
+ Some(Box::new(move |response| match &response {
+ pb::response::Response::Eth(pb::EthResponse {
+ response:
+ Some(Response::TypedMsgValue(pb::EthTypedMessageValueResponse {
+ root_object,
+ path,
+ })),
+ }) => {
+ assert_eq!(*root_object, RootObject::Domain as i32);
+ assert_eq!(path, &[0]);
+ let count = request_count_clone.get();
+ if count > 0 {
+ panic!("chain_id requested more than once");
+ }
+ request_count_clone.set(count + 1);
+ Ok(pb::request::Request::Eth(pb::EthRequest {
+ request: Some(Request::TypedMsgValue(pb::EthTypedMessageValueRequest {
+ value: chain_id_value(chain_id),
+ data_length: 0,
+ })),
+ }))
+ }
+ _ => panic!("unexpected response"),
+ }));
+ request_count
+ }
+
+ fn make_chain_id_only_request(chain_id: u64) -> pb::EthSignTypedMessageRequest {
+ pb::EthSignTypedMessageRequest {
+ chain_id,
+ keypath: vec![44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0],
+ types: vec![StructType {
+ name: "EIP712Domain".into(),
+ members: vec![mk_member("chainId", mk_sized_type(DataType::Uint, 32))],
+ }],
+ primary_type: DOMAIN_TYPE_NAME.into(),
+ host_nonce_commitment: None,
+ }
+ }
+
fn make_types() -> Vec<StructType> {
vec![
StructType {
@@ -764,9 +891,14 @@ mod tests {
}));
}
let mut mock_hal = TestingHal::new();
- eip712_sighash(&mut mock_hal, &typed_msg.types, typed_msg.primary_type)
- .await
- .unwrap();
+ eip712_sighash(
+ &mut mock_hal,
+ &typed_msg.types,
+ typed_msg.primary_type,
+ None,
+ )
+ .await
+ .unwrap();
mock_hal
}
@@ -1243,6 +1375,81 @@ mod tests {
);
}
+ #[test]
+ fn test_validate_identifier() {
+ for name in ["field", "_spender", "$value", "chainId", "value2", "from"] {
+ assert_eq!(validate_identifier(name), Ok(()));
+ }
+
+ for name in [
+ "",
+ "123amount",
+ "field:name",
+ "field\nname",
+ "field.name",
+ "foo[0]",
+ "uint256 amount",
+ "recipiént",
+ ] {
+ assert_eq!(validate_identifier(name), Err(Error::InvalidInput));
+ }
+ }
+
+ #[test]
+ fn test_validate_typed_msg_schema() {
+ let valid_types = vec![
+ StructType {
+ name: "EIP712Domain".into(),
+ members: vec![mk_member("name", mk_type(DataType::String))],
+ },
+ StructType {
+ name: "Inner".into(),
+ members: vec![mk_member("value", mk_type(DataType::String))],
+ },
+ StructType {
+ name: "Msg".into(),
+ members: vec![
+ mk_member("data", mk_struct_type("Inner")),
+ mk_member("items", mk_arr_type(mk_struct_type("Inner"))),
+ ],
+ },
+ ];
+ assert_eq!(validate_typed_msg_schema(&valid_types, "Msg"), Ok(()));
+
+ assert_eq!(
+ validate_typed_msg_schema(&valid_types, "Bad\nType"),
+ Err(Error::InvalidInput)
+ );
+
+ let mut types = valid_types.clone();
+ types[1].name = "Bad\nType".into();
+ assert_eq!(
+ validate_typed_msg_schema(&types, "Msg"),
+ Err(Error::InvalidInput)
+ );
+
+ let mut types = valid_types.clone();
+ types[2].members[0] = mk_member("field\nname", mk_type(DataType::String));
+ assert_eq!(
+ validate_typed_msg_schema(&types, "Msg"),
+ Err(Error::InvalidInput)
+ );
+
+ let mut types = valid_types.clone();
+ types[2].members[0] = mk_member("data", mk_struct_type("Bad\nType"));
+ assert_eq!(
+ validate_typed_msg_schema(&types, "Msg"),
+ Err(Error::InvalidInput)
+ );
+
+ let mut types = valid_types;
+ types[2].members[1] = mk_member("items", mk_arr_type(mk_struct_type("Bad\nType")));
+ assert_eq!(
+ validate_typed_msg_schema(&types, "Msg"),
+ Err(Error::InvalidInput)
+ );
+ }
+
#[test]
fn test_type_hash() {
assert_eq!(
@@ -1251,6 +1458,37 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_process_rejects_invalid_member_name() {
+ *crate::hww::MOCK_NEXT_REQUEST.0.borrow_mut() =
+ Some(Box::new(|_| panic!("value requested for invalid schema")));
+
+ let mut mock_hal = TestingHal::new();
+ let result = process(
+ &mut mock_hal,
+ &pb::EthSignTypedMessageRequest {
+ chain_id: 1,
+ keypath: vec![44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0],
+ types: vec![
+ StructType {
+ name: "EIP712Domain".into(),
+ members: vec![mk_member("name", mk_type(DataType::String))],
+ },
+ StructType {
+ name: "Msg".into(),
+ members: vec![mk_member("field\nname", mk_type(DataType::String))],
+ },
+ ],
+ primary_type: "Msg".into(),
+ host_nonce_commitment: None,
+ },
+ )
+ .await;
+
+ assert_eq!(result, Err(Error::InvalidInput));
+ assert!(mock_hal.ui.screens.is_empty());
+ }
+
/// Test computation of the domain separator, which is `hashStruct(domain)`.
#[async_test::test]
async fn test_domain_separator() {
@@ -1272,17 +1510,17 @@ mod tests {
}));
}
let mut mock_hal = TestingHal::new();
- let domain_separator = hash_struct(
- &mut mock_hal,
- &typed_msg.types,
- RootObject::Domain,
- "EIP712Domain",
- &[],
- &[],
- None,
- )
- .await
- .unwrap();
+ let context = HashContext {
+ types: &typed_msg.types,
+ root_object: RootObject::Domain,
+ path: &[],
+ formatted_path: &[],
+ title_suffix: None,
+ cached_chain_id: None,
+ };
+ let domain_separator = hash_struct(&mut mock_hal, "EIP712Domain", &context)
+ .await
+ .unwrap();
assert_eq!(
domain_separator,
b"\xf2\xce\xe3\x75\xfa\x42\xb4\x21\x43\x80\x40\x25\xfc\x44\x9d\xea\xfd\x50\xcc\x03\x1c\xa2\x57\xe0\xb1\x94\xa6\x50\xa9\x12\x09\x0f".to_vec());
@@ -1314,6 +1552,91 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_validate_chain_id_returns_none_if_missing() {
+ let result = validate_chain_id(&pb::EthSignTypedMessageRequest {
+ chain_id: 1,
+ types: vec![StructType {
+ name: "EIP712Domain".into(),
+ members: vec![mk_member("name", mk_type(DataType::String))],
+ }],
+ ..Default::default()
+ })
+ .await;
+ assert!(matches!(result, Ok(None)));
+ }
+
+ #[async_test::test]
+ async fn test_process_warns_if_chain_id_missing() {
+ mock_unlocked();
+ let typed_msg = alloc::rc::Rc::new(TypedMessage::new(
+ vec![StructType {
+ name: "EIP712Domain".into(),
+ members: vec![mk_member("name", mk_type(DataType::String))],
+ }],
+ DOMAIN_TYPE_NAME,
+ Object::Struct(vec![Object::String("test")]),
+ Object::Struct(vec![]),
+ ));
+ {
+ let typed_msg = typed_msg.clone();
+ *crate::hww::MOCK_NEXT_REQUEST.0.borrow_mut() = Some(Box::new(move |response| {
+ Ok(typed_msg.handle_host_response(&response).unwrap())
+ }));
+ }
+ let mut mock_hal = TestingHal::new();
+ let result = process(
+ &mut mock_hal,
+ &pb::EthSignTypedMessageRequest {
+ chain_id: 1,
+ keypath: vec![44 + HARDENED, 60 + HARDENED, 0 + HARDENED, 0, 0],
+ types: typed_msg.types.clone(),
+ primary_type: typed_msg.primary_type.into(),
+ host_nonce_commitment: None,
+ },
+ )
+ .await;
+
+ assert!(matches!(result, Ok(Response::Sign(_))));
+ assert_eq!(
+ mock_hal.ui.screens[0],
+ Screen::Confirm {
+ title: "Warning".into(),
+ body: "Typed data has no chain ID. Message is valid for every chain.".into(),
+ longtouch: false,
+ }
+ );
+ }
+
+ #[async_test::test]
+ async fn test_validate_chain_id_rejects_non_uint_type() {
+ let result = validate_chain_id(&pb::EthSignTypedMessageRequest {
+ chain_id: 1,
+ types: vec![StructType {
+ name: "EIP712Domain".into(),
+ members: vec![mk_member("chainId", mk_sized_type(DataType::Int, 32))],
+ }],
+ ..Default::default()
+ })
+ .await;
+ assert!(matches!(result, Err(Error::InvalidInput)));
+ }
+
+ #[async_test::test]
+ async fn test_validate_chain_id_reuses_cached_value() {
+ for (request_chain_id, chain_id) in [(1, Some(1)), (0, None)] {
+ mock_unlocked();
+ let request_count = setup_chain_id_responder_panicking_on_second_request(chain_id);
+ let result = process(
+ &mut TestingHal::new(),
+ &make_chain_id_only_request(request_chain_id),
+ )
+ .await;
+ assert!(matches!(result, Ok(Response::Sign(_))));
+ assert_eq!(request_count.get(), 1);
+ }
+ }
+
/// A typed data object which contains almost every type possible.
///
/// Reproduce the below sighash result by running the below with nodejs:
@@ -1797,9 +2120,14 @@ mod tests {
}));
}
let mut mock_hal = TestingHal::new();
- let sighash = eip712_sighash(&mut mock_hal, &typed_msg.types, typed_msg.primary_type)
- .await
- .unwrap();
+ let sighash = eip712_sighash(
+ &mut mock_hal,
+ &typed_msg.types,
+ typed_msg.primary_type,
+ None,
+ )
+ .await
+ .unwrap();
assert_eq!(
sighash,
*b"\x0e\xfe\x31\xa8\x81\x9b\x6c\x38\x1c\x9e\x97\xcf\xd2\x99\x5a\xa6\xf2\x1e\x4a\x72\x87\x9a\xc1\x31\xb2\xf6\x48\xd0\x83\x28\x1c\x83",
@@ -1883,6 +2211,7 @@ mod tests {
&mut TestingHal::new(),
&typed_msg.types,
typed_msg.primary_type,
+ None,
)
.await
.unwrap();
@@ -2005,6 +2334,7 @@ mod tests {
&mut TestingHal::new(),
&typed_msg.types,
typed_msg.primary_type,
+ None,
)
.await
.unwrap();
@@ -2029,10 +2359,14 @@ mod tests {
Ok(typed_msg_clone.handle_host_response(&response).unwrap())
}));
let mut mock_hal = TestingHal::new();
- let streaming_sighash =
- eip712_sighash(&mut mock_hal, &typed_msg.types, typed_msg.primary_type)
- .await
- .unwrap();
+ let streaming_sighash = eip712_sighash(
+ &mut mock_hal,
+ &typed_msg.types,
+ typed_msg.primary_type,
+ None,
+ )
+ .await
+ .unwrap();
assert_eq!(
streaming_sighash, expected,
"streaming sighash mismatch for: {}",
@@ -2065,10 +2399,14 @@ mod tests {
Ok(typed_msg_clone.handle_host_response(&response).unwrap())
}));
let mut mock_hal = TestingHal::new();
- let sighash =
- eip712_sighash(&mut mock_hal, &typed_msg.types, typed_msg.primary_type)
- .await
- .unwrap();
+ let sighash = eip712_sighash(
+ &mut mock_hal,
+ &typed_msg.types,
+ typed_msg.primary_type,
+ None,
+ )
+ .await
+ .unwrap();
assert_eq!(
sighash, expected,
"sighash mismatch for: {}",
@@ -2125,11 +2463,25 @@ mod tests {
"signature mismatch for: {}",
tc.description
);
- let mut expected_screens = vec![Screen::Confirm {
+ let mut expected_screens = Vec::new();
+ if !get_type(&types, DOMAIN_TYPE_NAME)
+ .unwrap()
+ .members
+ .iter()
+ .any(|member| member.name == "chainId")
+ {
+ expected_screens.push(Screen::Confirm {
+ title: "Warning".into(),
+ body: "Typed data has no chain ID. Message is valid for every chain."
+ .into(),
+ longtouch: false,
+ });
+ }
+ expected_screens.push(Screen::Confirm {
title: "Ethereum".into(),
body: address.clone(),
longtouch: false,
- }];
+ });
expected_screens.extend(tc.expected_screens.iter().map(|(title, body)| {
Screen::Confirm {
title: title.clone(),
@@ -2176,7 +2528,13 @@ mod tests {
}));
}
let mut mock_hal = TestingHal::new();
- let result = eip712_sighash(&mut mock_hal, &typed_msg.types, typed_msg.primary_type).await;
+ let result = eip712_sighash(
+ &mut mock_hal,
+ &typed_msg.types,
+ typed_msg.primary_type,
+ None,
+ )
+ .await;
assert_eq!(result, Err(Error::InvalidInput));
}
@@ -2205,7 +2563,13 @@ mod tests {
}));
}
let mut mock_hal = TestingHal::new();
- let result = eip712_sighash(&mut mock_hal, &typed_msg.types, typed_msg.primary_type).await;
+ let result = eip712_sighash(
+ &mut mock_hal,
+ &typed_msg.types,
+ typed_msg.primary_type,
+ None,
+ )
+ .await;
assert_eq!(result, Err(Error::InvalidInput));
}
@@ -2251,7 +2615,7 @@ mod tests {
},
];
let mut mock_hal = TestingHal::new();
- let result = eip712_sighash(&mut mock_hal, &types, "Msg").await;
+ let result = eip712_sighash(&mut mock_hal, &types, "Msg", None).await;
assert_eq!(result, Err(Error::InvalidInput));
}
@@ -2299,7 +2663,7 @@ mod tests {
},
];
let mut mock_hal = TestingHal::new();
- let result = eip712_sighash(&mut mock_hal, &types, "Msg").await;
+ let result = eip712_sighash(&mut mock_hal, &types, "Msg", None).await;
assert_eq!(result, Err(Error::InvalidInput));
}
}
diff --git a/src/rust/bitbox02-rust/src/workflow/verify_message.rs b/src/rust/bitbox02-rust/src/workflow/verify_message.rs
index c9d3e6d..dc53ce1 100644
--- a/src/rust/bitbox02-rust/src/workflow/verify_message.rs
+++ b/src/rust/bitbox02-rust/src/workflow/verify_message.rs
@@ -39,8 +39,11 @@ pub async fn verify(
if ascii::is_printable_ascii(msg, ascii::Charset::AllNewline) {
// The message is all ascii and printable.
let msg = core::str::from_utf8(msg).unwrap();
+ if msg.is_empty() {
+ return Err(Error::InvalidInput);
+ }
- let pages: Vec<&str> = msg.split('\n').filter(|line| !line.is_empty()).collect();
+ let pages: Vec<&str> = msg.split('\n').collect();
if pages.is_empty() {
return Err(Error::InvalidInput);
}
@@ -76,3 +79,111 @@ pub async fn verify(
Ok(())
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ use crate::hal::testing::TestingHal;
+ use crate::hal::testing::ui::Screen;
+
+ #[async_test::test]
+ async fn test_verify_multiline_text() {
+ let mut hal = TestingHal::new();
+ assert!(
+ verify(&mut hal, "Sign message", "Sign", b"A\nB", true)
+ .await
+ .is_ok()
+ );
+
+ assert_eq!(
+ hal.ui.screens,
+ vec![
+ Screen::Confirm {
+ title: "Sign 1/2".into(),
+ body: "A".into(),
+ longtouch: false,
+ },
+ Screen::Confirm {
+ title: "Sign 2/2".into(),
+ body: "B".into(),
+ longtouch: true,
+ },
+ ]
+ );
+ }
+
+ #[async_test::test]
+ async fn test_verify_blank_lines() {
+ let mut hal = TestingHal::new();
+ assert!(
+ verify(&mut hal, "Sign message", "Sign", b"A\n\nB", true)
+ .await
+ .is_ok()
+ );
+ assert_eq!(
+ hal.ui.screens,
+ vec![
+ Screen::Confirm {
+ title: "Sign 1/3".into(),
+ body: "A".into(),
+ longtouch: false,
+ },
+ Screen::Confirm {
+ title: "Sign 2/3".into(),
+ body: "".into(),
+ longtouch: false,
+ },
+ Screen::Confirm {
+ title: "Sign 3/3".into(),
+ body: "B".into(),
+ longtouch: true,
+ },
+ ]
+ );
+
+ let mut hal = TestingHal::new();
+ assert!(
+ verify(&mut hal, "Sign message", "Sign", b"\nA", true)
+ .await
+ .is_ok()
+ );
+ assert_eq!(
+ hal.ui.screens,
+ vec![
+ Screen::Confirm {
+ title: "Sign 1/2".into(),
+ body: "".into(),
+ longtouch: false,
+ },
+ Screen::Confirm {
+ title: "Sign 2/2".into(),
+ body: "A".into(),
+ longtouch: true,
+ },
+ ]
+ );
+
+ let mut hal = TestingHal::new();
+ assert!(
+ verify(&mut hal, "Sign message", "Sign", b"A\n", true)
+ .await
+ .is_ok()
+ );
+ assert_eq!(
+ hal.ui.screens,
+ vec![
+ Screen::Confirm {
+ title: "Sign 1/2".into(),
+ body: "A".into(),
+ longtouch: false,
+ },
+ Screen::Confirm {
+ title: "Sign 2/2".into(),
+ body: "".into(),
+ longtouch: true,
+ },
+ ]
+ );
+ }
+}
diff --git a/src/usb/class/hid/hid.c b/src/usb/class/hid/hid.c
index 3f22d7d..7d4a00a 100644
--- a/src/usb/class/hid/hid.c
+++ b/src/usb/class/hid/hid.c
@@ -5,6 +5,7 @@
#if !defined(TESTING)
#include "usb_protocol.h"
#endif
+#include <assert.h>
#include <string.h>
/**
@@ -188,15 +189,22 @@ int32_t hid_req(
case 0x0B: /* Set Protocol */
func_data->protocol = req->wValue;
return usbdc_xfer(ep, NULL, 0, 0);
- case USB_REQ_HID_SET_REPORT:
+ case USB_REQ_HID_SET_REPORT: {
+ static_assert(
+ USB_HID_REPORT_OUT_SIZE == USB_REPORT_SIZE,
+ "USB_HID_REPORT_OUT_SIZE must match USB_REPORT_SIZE");
+ if (len > USB_HID_REPORT_OUT_SIZE) {
+ return ERR_INVALID_ARG;
+ }
if (USB_SETUP_STAGE == stage) {
return usbdc_xfer(ep, ctrl_buf, len, false);
- } else {
- if (NULL != func_data->hid_set_report) {
- func_data->hid_set_report(ctrl_buf, len);
- }
- return ERR_NONE;
}
+
+ if (NULL != func_data->hid_set_report) {
+ func_data->hid_set_report(ctrl_buf, len);
+ }
+ return ERR_NONE;
+ }
default:
return ERR_INVALID_ARG;
}
diff --git a/test/simulator-graphical-bb03/Cargo.lock b/test/simulator-graphical-bb03/Cargo.lock
index bd91703..a23d59d 100644
--- a/test/simulator-graphical-bb03/Cargo.lock
+++ b/test/simulator-graphical-bb03/Cargo.lock
@@ -1012,6 +1012,7 @@ dependencies = [
"fiat-crypto",
"rustc_version 0.4.1",
"subtle",
+ "zeroize",
]
[[package]]
@@ -1102,6 +1103,7 @@ dependencies = [
"sha2",
"signature",
"subtle",
+ "zeroize",
]
[[package]]
@@ -3886,6 +3888,7 @@ checksum = "c7e468321c81fb07fa7f4c636c3972b9100f0346e5b6a9f2bd0603a52f7ed277"
dependencies = [
"curve25519-dalek",
"rand_core 0.6.4",
+ "zeroize",
]
[[package]]
diff --git a/test/simulator-graphical/Cargo.lock b/test/simulator-graphical/Cargo.lock
index 00e4840..aedc75f 100644
--- a/test/simulator-graphical/Cargo.lock
+++ b/test/simulator-graphical/Cargo.lock
@@ -954,6 +954,7 @@ dependencies = [
"fiat-crypto",
"rustc_version 0.4.0",
"subtle",
+ "zeroize",
]
[[package]]
@@ -1050,6 +1051,7 @@ dependencies = [
"sha2",
"signature",
"subtle",
+ "zeroize",
]
[[package]]
@@ -3946,6 +3948,7 @@ checksum = "fb66477291e7e8d2b0ff1bcb900bf29489a9692816d79874bea351e7a8b6de96"
dependencies = [
"curve25519-dalek",
"rand_core",
+ "zeroize",
]
[[package]]
diff --git a/test/unit-test/test_memory.c b/test/unit-test/test_memory.c
index d947e13..df178f2 100644
--- a/test/unit-test/test_memory.c
+++ b/test/unit-test/test_memory.c
@@ -430,6 +430,44 @@ static void _test_memory_reset_hww_ble(void** state)
assert_true(memory_reset_hww());
}
+static void _test_memory_get_ble_bond_db(void** state)
+{
+ (void)state;
+ const uint8_t bond_db[] = {0x01, 0x02, 0x03};
+ chunk_shared_t shared_chunk = {0};
+ shared_chunk.fields.ble_bond_db_len = sizeof(bond_db);
+ memcpy(shared_chunk.fields.ble_bond_db, bond_db, sizeof(bond_db));
+ will_return(__wrap_memory_read_shared_bootdata_fake, shared_chunk.bytes);
+
+ uint8_t data[MEMORY_BLE_BOND_DB_LEN] = {0};
+ assert_int_equal(memory_get_ble_bond_db(data), sizeof(bond_db));
+ assert_memory_equal(data, bond_db, sizeof(bond_db));
+}
+
+static void _test_memory_get_ble_bond_db_invalid_negative_length(void** state)
+{
+ (void)state;
+ chunk_shared_t shared_chunk = {0};
+ shared_chunk.fields.ble_bond_db_len = -2;
+ memset(shared_chunk.fields.ble_bond_db, 0x42, sizeof(shared_chunk.fields.ble_bond_db));
+ will_return(__wrap_memory_read_shared_bootdata_fake, shared_chunk.bytes);
+
+ uint8_t data[MEMORY_BLE_BOND_DB_LEN];
+ uint8_t expected[MEMORY_BLE_BOND_DB_LEN];
+ memset(data, 0xa5, sizeof(data));
+ memset(expected, 0xa5, sizeof(expected));
+
+ assert_int_equal(memory_get_ble_bond_db(data), -1);
+ assert_memory_equal(data, expected, sizeof(data));
+}
+
+static void _test_memory_set_ble_bond_db_invalid_negative_length(void** state)
+{
+ (void)state;
+ const uint8_t bond_db[] = {0x01};
+ assert_false(memory_set_ble_bond_db(bond_db, -1));
+}
+
static void _test_memory_get_device_name_default(void** state)
{
char name_out[MEMORY_DEVICE_MAX_LEN_WITH_NULL] = {0};
@@ -633,6 +671,9 @@ int main(void)
cmocka_unit_test(_test_memory_set_mnemonic_passphrase_enabled),
cmocka_unit_test(_test_memory_reset_hww),
cmocka_unit_test(_test_memory_reset_hww_ble),
+ cmocka_unit_test(_test_memory_get_ble_bond_db),
+ cmocka_unit_test(_test_memory_get_ble_bond_db_invalid_negative_length),
+ cmocka_unit_test(_test_memory_set_ble_bond_db_invalid_negative_length),
cmocka_unit_test(_test_memory_get_device_name_default),
cmocka_unit_test(_test_memory_get_device_name_default_bluetooth),
cmocka_unit_test(_test_memory_get_device_name_invalid),
diff --git a/versions.json b/versions.json
index 67dc2cc..7b6ba91 100644
--- a/versions.json
+++ b/versions.json
@@ -1,4 +1,4 @@
{
- "firmware": "v9.26.1",
- "bootloader": "v1.1.2"
+ "firmware": "v9.26.3",
+ "bootloader": "v1.1.3"
}
Risk score
Our methodology →Why this scored 76/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.