What changed, and why it matters
This commit replaces floating-point progress calculations with integer math to save firmware space. It removes the use of f32 arithmetic for progress bars in Bitcoin transaction signing and Bluetooth firmware upgrades. There is no direct security vulnerability in the change itself; it is a code-quality and size-reduction refactor. However, it introduces new integer arithmetic paths that must keep numerator/denominator assumptions valid to avoid incorrect progress display.
No immediate action required. Reviewers should verify that all future call sites of `set_fraction` enforce `denominator > 0` and `numerator <= denominator`, and that the 64-bit intermediate in `progress.c` remains sufficient for expected numerator ranges. Consider adding unit tests for edge cases (zero denominator, numerator > denominator, large values).
Security signals we found
Removal of floating-point arithmetic reduces attack surface related to soft-float emulation bugs
New integer fraction API introduces denominator-non-zero and numerator-bound assumptions
Call sites use checked arithmetic to prevent overflow in progress computation
C-side progress width uses 64-bit intermediate to avoid overflow when multiplying SCREEN_WIDTH * numerator
Evidence from the diff
The commit refactors the progress bar API from set(progress: f32) to set_fraction(numerator: u32, denominator: u32). The C progress component now stores filled_width as a uint16_t and computes it as SCREEN_WIDTH * numerator / denominator using 64-bit intermediate math. Rust call sites in Bitcoin signing (signtx.rs) and Bluetooth upgrade (bluetooth.rs) now use checked integer arithmetic to compute progress fractions. The change eliminates soft-float compiler builtins, saving 1352 bytes. The primary security-relevant concern is ensuring the contract denominator != 0 and numerator <= denominator is maintained across all call sites; the diff shows callers generally preserve this, with explicit checked_add/checked_mul guards in the more complex prevtx path.
Changed components
src/rust/bitbox-hal/src/ui.rssrc/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rssrc/rust/bitbox02-rust/src/hww/api/bluetooth.rssrc/ui/components/progress.csrc/ui/components/progress.hInspect captured patch +52 / −37
diff --git a/src/rust/bitbox-hal/src/ui.rs b/src/rust/bitbox-hal/src/ui.rs
index 1c300b0..29eb0ab 100644
--- a/src/rust/bitbox-hal/src/ui.rs
+++ b/src/rust/bitbox-hal/src/ui.rs
@@ -62,8 +62,9 @@ pub enum CanCancel {
}
pub trait Progress {
- /// Set progress. `progress` should be in the range `[0.0, 1.0]`.
- fn set(&mut self, progress: f32);
+ /// Set progress as a fraction. `denominator` must be non-zero and
+ /// `numerator <= denominator`.
+ fn set_fraction(&mut self, numerator: u32, denominator: u32);
}
pub trait Empty {}
diff --git a/src/rust/bitbox02-rust/src/hal/testing/ui.rs b/src/rust/bitbox02-rust/src/hal/testing/ui.rs
index 6437fe4..2fdbe41 100644
--- a/src/rust/bitbox02-rust/src/hal/testing/ui.rs
+++ b/src/rust/bitbox02-rust/src/hal/testing/ui.rs
@@ -73,7 +73,7 @@ pub struct TestingUi<'a> {
pub struct NoopProgress;
impl Progress for NoopProgress {
- fn set(&mut self, _progress: f32) {}
+ fn set_fraction(&mut self, _numerator: u32, _denominator: u32) {}
}
pub struct NoopEmpty;
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 7d71735..d606ccf 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bitcoin/signtx.rs
@@ -360,15 +360,26 @@ async fn handle_prevtx(
let mut hasher = Sha256::new();
hasher.update(prevtx_init.version.to_le_bytes());
+ let prevtx_total_ios = prevtx_init
+ .num_inputs
+ .checked_add(prevtx_init.num_outputs)
+ .ok_or(Error::InvalidInput)?;
+ let prevtx_progress_denominator = num_inputs
+ .checked_mul(prevtx_total_ios)
+ .ok_or(Error::InvalidInput)?;
+ let prevtx_progress_input_start = input_index
+ .checked_mul(prevtx_total_ios)
+ .ok_or(Error::InvalidInput)?;
+
hasher.update(serialize(&VarInt(prevtx_init.num_inputs as u64)));
for prevtx_input_index in 0..prevtx_init.num_inputs {
// Update progress.
- progress_component.set({
- let step = 1f32 / (num_inputs as f32);
- let subprogress: f32 = (prevtx_input_index as f32)
- / (prevtx_init.num_inputs + prevtx_init.num_outputs) as f32;
- (input_index as f32 + subprogress) * step
- });
+ progress_component.set_fraction(
+ prevtx_progress_input_start
+ .checked_add(prevtx_input_index)
+ .ok_or(Error::InvalidInput)?,
+ prevtx_progress_denominator,
+ );
let prevtx_input = get_prevtx_input(input_index, prevtx_input_index, next_response).await?;
hasher.update(prevtx_input.prev_out_hash.as_slice());
@@ -383,12 +394,13 @@ async fn handle_prevtx(
hasher.update(serialize(&VarInt(prevtx_init.num_outputs as u64)));
for prevtx_output_index in 0..prevtx_init.num_outputs {
// Update progress.
- progress_component.set({
- let step = 1f32 / (num_inputs as f32);
- let subprogress: f32 = (prevtx_init.num_inputs + prevtx_output_index) as f32
- / (prevtx_init.num_inputs + prevtx_init.num_outputs) as f32;
- (input_index as f32 + subprogress) * step
- });
+ progress_component.set_fraction(
+ prevtx_progress_input_start
+ .checked_add(prevtx_init.num_inputs)
+ .and_then(|progress| progress.checked_add(prevtx_output_index))
+ .ok_or(Error::InvalidInput)?,
+ prevtx_progress_denominator,
+ );
let prevtx_output =
get_prevtx_output(input_index, prevtx_output_index, next_response).await?;
@@ -760,7 +772,7 @@ async fn _process(
progress_component
.as_mut()
.unwrap()
- .set((input_index as f32) / (request.num_inputs as f32));
+ .set_fraction(input_index, request.num_inputs);
let tx_input = get_tx_input(input_index, &mut next_response).await?;
let script_config_account = validated_script_configs
@@ -847,7 +859,7 @@ async fn _process(
}
// The progress for loading the inputs is 100%.
- progress_component.as_mut().unwrap().set(1.);
+ progress_component.as_mut().unwrap().set_fraction(1, 1);
let hash_prevouts = hasher_prevouts.finalize();
let hash_sequence = hasher_sequence.finalize();
@@ -1317,7 +1329,7 @@ async fn _process(
// Update progress.
if let Some(ref mut c) = progress_component {
- c.set((input_index + 1) as f32 / (request.num_inputs as f32));
+ c.set_fraction(input_index + 1, request.num_inputs);
}
}
diff --git a/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs b/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs
index 38c5258..4c55f7d 100644
--- a/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs
+++ b/src/rust/bitbox02-rust/src/hww/api/bluetooth.rs
@@ -97,7 +97,7 @@ async fn process_upgrade_helper<M: Memory>(
memory.ble_firmware_flash_chunk(inactive_slot, chunk_index, &chunk)?;
// Update progress.
- progress.set((chunk_index + 1) as f32 / (num_chunks as f32));
+ progress.set_fraction(chunk_index + 1, num_chunks);
}
let firmware_hash: [u8; 32] = firmware_hasher.finalize().into();
@@ -242,8 +242,8 @@ mod tests {
}
impl Progress for TestProgress {
- fn set(&mut self, progress: f32) {
- self.values.push(progress);
+ fn set_fraction(&mut self, numerator: u32, denominator: u32) {
+ self.values.push(numerator as f32 / denominator as f32);
}
}
diff --git a/src/rust/bitbox02-sys/build.rs b/src/rust/bitbox02-sys/build.rs
index 67998c8..c74fc68 100644
--- a/src/rust/bitbox02-sys/build.rs
+++ b/src/rust/bitbox02-sys/build.rs
@@ -131,7 +131,7 @@ const ALLOWLIST_FNS: &[&str] = &[
"platform_product",
"printf",
"progress_create",
- "progress_set",
+ "progress_set_fraction",
"random_32_bytes_mcu",
"random_32_bytes",
"random_fake_reset",
diff --git a/src/rust/bitbox02/src/hal/ui.rs b/src/rust/bitbox02/src/hal/ui.rs
index 440bfbf..3dd251b 100644
--- a/src/rust/bitbox02/src/hal/ui.rs
+++ b/src/rust/bitbox02/src/hal/ui.rs
@@ -19,8 +19,8 @@ pub struct BitBox02Progress {
}
impl HalProgress for BitBox02Progress {
- fn set(&mut self, progress: f32) {
- crate::ui::progress_set(&mut self.component, progress);
+ fn set_fraction(&mut self, numerator: u32, denominator: u32) {
+ crate::ui::progress_set_fraction(&mut self.component, numerator, denominator);
}
}
diff --git a/src/rust/bitbox02/src/ui/ui.rs b/src/rust/bitbox02/src/ui/ui.rs
index 32904ca..9063e01 100644
--- a/src/rust/bitbox02/src/ui/ui.rs
+++ b/src/rust/bitbox02/src/ui/ui.rs
@@ -781,8 +781,8 @@ pub fn progress_create(title: &str) -> Component {
}
}
-pub fn progress_set(component: &mut Component, progress: f32) {
- unsafe { bitbox02_sys::progress_set(component.component, progress) }
+pub fn progress_set_fraction(component: &mut Component, numerator: u32, denominator: u32) {
+ unsafe { bitbox02_sys::progress_set_fraction(component.component, numerator, denominator) }
}
pub fn empty_create() -> Component {
diff --git a/src/rust/bitbox02/src/ui/ui_stub.rs b/src/rust/bitbox02/src/ui/ui_stub.rs
index c10d4ac..de758ca 100644
--- a/src/rust/bitbox02/src/ui/ui_stub.rs
+++ b/src/rust/bitbox02/src/ui/ui_stub.rs
@@ -93,7 +93,7 @@ pub fn progress_create(_title: &str) -> Component {
Component { is_pushed: false }
}
-pub fn progress_set(_component: &mut Component, _progress: f32) {}
+pub fn progress_set_fraction(_component: &mut Component, _numerator: u32, _denominator: u32) {}
pub fn empty_create() -> Component {
Component { is_pushed: false }
diff --git a/src/rust/bitbox02/src/ui/ui_stub_c_unit_tests.rs b/src/rust/bitbox02/src/ui/ui_stub_c_unit_tests.rs
index e53ff8e..2958911 100644
--- a/src/rust/bitbox02/src/ui/ui_stub_c_unit_tests.rs
+++ b/src/rust/bitbox02/src/ui/ui_stub_c_unit_tests.rs
@@ -116,7 +116,7 @@ pub fn progress_create(_title: &str) -> Component {
Component { is_pushed: false }
}
-pub fn progress_set(_component: &mut Component, _progress: f32) {}
+pub fn progress_set_fraction(_component: &mut Component, _numerator: u32, _denominator: u32) {}
pub fn empty_create() -> Component {
Component { is_pushed: false }
diff --git a/src/rust/bitbox03/src/ui.rs b/src/rust/bitbox03/src/ui.rs
index f6830ee..fcf842b 100644
--- a/src/rust/bitbox03/src/ui.rs
+++ b/src/rust/bitbox03/src/ui.rs
@@ -38,7 +38,7 @@ impl<Timer> Drop for ScreenGuard<'_, Timer> {
}
impl hal::ui::Progress for BitBox03UiProgress {
- fn set(&mut self, _progress: f32) {
+ fn set_fraction(&mut self, _numerator: u32, _denominator: u32) {
todo!()
}
}
diff --git a/src/ui/components/progress.c b/src/ui/components/progress.c
index 8e8ff32..f6f0ea0 100644
--- a/src/ui/components/progress.c
+++ b/src/ui/components/progress.c
@@ -10,15 +10,17 @@
#include <string.h>
typedef struct {
- float progress;
+ uint16_t filled_width;
} data_t;
static void _render(component_t* component)
{
const data_t* data = (const data_t*)component->data;
const uint16_t bar_height = 5;
- UG_FillFrame(
- 0, SCREEN_HEIGHT - bar_height, SCREEN_WIDTH * data->progress, SCREEN_HEIGHT, C_WHITE);
+ if (data->filled_width > 0) {
+ UG_FillFrame(
+ 0, SCREEN_HEIGHT - bar_height, data->filled_width - 1, SCREEN_HEIGHT - 1, C_WHITE);
+ }
ui_util_component_render_subcomponents(component);
}
@@ -49,8 +51,8 @@ component_t* progress_create(const char* title)
return component;
}
-void progress_set(component_t* component, float progress)
+void progress_set_fraction(component_t* component, uint32_t numerator, uint32_t denominator)
{
data_t* data = (data_t*)component->data;
- data->progress = progress;
+ data->filled_width = (uint16_t)((uint64_t)SCREEN_WIDTH * numerator / denominator);
}
diff --git a/src/ui/components/progress.h b/src/ui/components/progress.h
index 9ea2425..caec5b3 100644
--- a/src/ui/components/progress.h
+++ b/src/ui/components/progress.h
@@ -11,9 +11,9 @@
component_t* progress_create(const char* title);
/**
- * Set the progress.
- * @param[in] progress value must be in [0, 1].
+ * Set the progress as an exact fraction.
+ * @param[in] denominator must be non-zero.
*/
-void progress_set(component_t* component, float progress);
+void progress_set_fraction(component_t* component, uint32_t numerator, uint32_t denominator);
#endif
Why this scored 19/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.