What changed, and why it matters
This commit fixes a fee-warning gap in the BitBox02 Bitcoin signing flow. Previously, transactions that only sent money back to yourself (change outputs) or only carried an OP_RETURN message had no 'send amount' to compare the fee against, so the device would not warn even if the fee was huge. The patch now compares the fee to the total value of all transaction inputs in those cases, and still warns the user when the fee exceeds 10% of that total. It also updates the on-screen wording to say whether the percentage is relative to the send amount or to all inputs.
Treat as a security-hardening fix and include in the next firmware release. No immediate incident response is indicated, but users should be advised to update so that abnormal all-change/OP_RETURN fees trigger the usual on-device warning.
Security signals we found
Missing fee sanity check for edge-case transaction shapes
User-interface warning now covers all-change and OP_RETURN-only transactions
Fee denominator changed from external outputs to verified input total for no-external-output transactions
Regression tests added for high-fee and below-threshold cases
Evidence from the diff
In bitcoin/signtx.rs, when outputs_sum_out is zero (all-change or OP_RETURN-only), the code now computes fee_percentage as 100 * fee / inputs_sum_pass1 and passes FeePercentageBasis::TotalInputs to the workflow. Otherwise it keeps the existing send-amount basis. workflow/transaction.rs adds the FeePercentageBasis enum and a new verify_total_fee_maybe_warn_with_basis helper that selects the warning body text (‘the send amount’ vs ‘of all inputs’). The FEE_WARNING_THRESHOLD remains 10%. Regression tests cover both the above-threshold warning path and the below-threshold no-warning path.
Changed components
src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rssrc/rust/bitbox02-rust/src/workflow/transaction.rsBitBox02 Bitcoin transaction signing UI flowInspect captured patch +124 / −8
### src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
@@ -1160,16 +1160,28 @@ async fn _process(
let fee: u64 = total_out
.checked_sub(outputs_sum_out)
.ok_or(Error::InvalidInput)?;
- let fee_percentage: Option<f64> = if outputs_sum_out == 0 {
- None
+ let (fee_percentage, fee_percentage_basis) = if outputs_sum_out == 0 {
+ // All-change and OP_RETURN-only transactions have no send amount to use as a denominator.
+ // Compare the fee to the verified input total instead, so consuming a large share of the
+ // inputs as fees still triggers the warning.
+ let fee_percentage = if inputs_sum_pass1 == 0 {
+ None
+ } else {
+ Some(100. * (fee as f64) / (inputs_sum_pass1 as f64))
+ };
+ (fee_percentage, transaction::FeePercentageBasis::TotalInputs)
} else {
- Some(100. * (fee as f64) / (outputs_sum_out as f64))
+ (
+ Some(100. * (fee as f64) / (outputs_sum_out as f64)),
+ transaction::FeePercentageBasis::SendAmount,
+ )
};
- transaction::verify_total_fee_maybe_warn(
+ transaction::verify_total_fee_maybe_warn_with_basis(
hal,
&format_amount(coin_params, format_unit, total_out)?,
&format_amount(coin_params, format_unit, fee)?,
fee_percentage,
+ fee_percentage_basis,
)
.await?;
hal.ui().status("Transaction\nconfirmed", true).await;
@@ -2584,6 +2596,73 @@ mod tests {
);
}
+ #[async_test::test]
+ async fn test_high_fee_warning_total_inputs() {
+ for with_op_return in [false, true] {
+ let transaction =
+ alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
+ {
+ let mut tx = transaction.borrow_mut();
+ tx.outputs.retain(|output| output.ours);
+ if with_op_return {
+ tx.outputs.push(pb::BtcSignOutputRequest {
+ r#type: pb::BtcOutputType::OpReturn as _,
+ payload: b"metadata".to_vec(),
+ ..Default::default()
+ });
+ }
+ }
+ mock_host_responder(transaction.clone());
+ mock_unlocked();
+ let init_request = transaction.borrow().init_request();
+
+ let mut mock_hal = TestingHal::new();
+ assert!(process(&mut mock_hal, &init_request).await.is_ok());
+
+ assert!(mock_hal.ui.screens.contains(&Screen::TotalFee {
+ total: "13.39999900 BTC".into(),
+ fee: "13.39999900 BTC".into(),
+ longtouch: false,
+ }));
+ assert!(
+ mock_hal
+ .ui
+ .contains_confirm("High fee", "The fee is 66.0%\nof all inputs.\nProceed?")
+ );
+ assert_eq!(
+ mock_hal.ui.contains_confirm("OP_RETURN", "metadata"),
+ with_op_return
+ );
+ }
+ }
+
+ #[async_test::test]
+ async fn test_no_high_fee_warning_total_inputs_below_threshold() {
+ let transaction =
+ alloc::rc::Rc::new(core::cell::RefCell::new(Transaction::new(pb::BtcCoin::Btc)));
+ {
+ let mut tx = transaction.borrow_mut();
+ tx.outputs.retain(|output| output.ours);
+ tx.outputs.truncate(1);
+ tx.outputs[0].value = 2_000_000_000;
+ }
+ mock_host_responder(transaction.clone());
+ mock_unlocked();
+ let init_request = transaction.borrow().init_request();
+
+ let mut mock_hal = TestingHal::new();
+ assert!(process(&mut mock_hal, &init_request).await.is_ok());
+
+ assert!(mock_hal.ui.screens.contains(&Screen::TotalFee {
+ total: "0.30000000 BTC".into(),
+ fee: "0.30000000 BTC".into(),
+ longtouch: true,
+ }));
+ assert!(!mock_hal.ui.screens.iter().any(|screen| {
+ matches!(screen, Screen::Confirm { title, .. } if title == "High fee")
+ }));
+ }
+
// Test a P2TR output. It is not part of the default test transaction because Taproot is not
// active on Litecoin yet.
#[async_test::test]
### src/rust/bitbox02-rust/src/workflow/transaction.rs
@@ -11,25 +11,62 @@ fn format_percentage(p: f64) -> String {
util::decimal::format_no_trim(int, 1)
}
+/// The denominator used to calculate and describe a transaction's fee percentage.
+pub enum FeePercentageBasis {
+ /// The amount sent to non-change/send outputs.
+ SendAmount,
+ /// The sum of the transaction's verified input values.
+ TotalInputs,
+}
+
+impl FeePercentageBasis {
+ fn warning_message(&self, fee_percentage: &str) -> String {
+ match self {
+ FeePercentageBasis::SendAmount => {
+ format!("The fee is {}%\nthe send amount.\nProceed?", fee_percentage)
+ }
+ FeePercentageBasis::TotalInputs => {
+ format!("The fee is {}%\nof all inputs.\nProceed?", fee_percentage)
+ }
+ }
+ }
+}
+
pub async fn verify_total_fee_maybe_warn(
hal: &mut impl crate::hal::Hal,
total: &str,
fee: &str,
fee_percentage: Option<f64>,
+) -> Result<(), UserAbort> {
+ verify_total_fee_maybe_warn_with_basis(
+ hal,
+ total,
+ fee,
+ fee_percentage,
+ FeePercentageBasis::SendAmount,
+ )
+ .await
+}
+
+pub async fn verify_total_fee_maybe_warn_with_basis(
+ hal: &mut impl crate::hal::Hal,
+ total: &str,
+ fee: &str,
+ fee_percentage: Option<f64>,
+ fee_percentage_basis: FeePercentageBasis,
) -> Result<(), UserAbort> {
const FEE_WARNING_THRESHOLD: f64 = 10.;
let fee_percentage = fee_percentage.filter(|&f| f >= FEE_WARNING_THRESHOLD);
let longtouch = fee_percentage.is_none();
hal.ui().verify_total_fee(total, fee, longtouch).await?;
if let Some(fee_percentage) = fee_percentage {
+ let warning_message =
+ fee_percentage_basis.warning_message(&format_percentage(fee_percentage));
hal.ui()
.confirm(&ConfirmParams {
title: "High fee",
- body: &format!(
- "The fee is {}%\nthe send amount.\nProceed?",
- format_percentage(fee_percentage)
- ),
+ body: &warning_message,
longtouch: true,
..Default::default()
})Why this scored 40/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.