fix(legacy): Avoid hypothetical division by zero.
What the developer wrote
Authored by Andrew Kozlik
fix(legacy): Avoid hypothetical division by zero.
[no changelog]
What changed, and why it matters
This commit fixes a theoretical 'division by zero' bug in the progress-bar code shown on the Trezor hardware wallet during Bitcoin transaction signing. The bug could only crash the device if internal counters used to calculate progress ever became zero, which the developers describe as 'hypothetical.' It does not affect cryptographic signing itself or expose funds.
No urgent action required. Treat as routine hardening. Users should keep firmware updated through normal vendor releases. If auditing, verify that `progress_substeps`, `progress_steps`, and `progress_midpoint` are always initialized to non-zero before `report_progress()` is called, making this patch a defense-in-depth measure.
Security signals we found
Defensive fix for division-by-zero fault in firmware UI progress calculation
No cryptographic or transaction-validation logic changed
Commit message explicitly calls the scenario 'hypothetical'
No changelog entry suggests vendor does not treat this as a user-visible security issue
Evidence from the diff
The patch introduces a helper interpolate(value, num, den) in legacy/firmware/signing.c that returns value when den == 0, otherwise computes (value * num) / den. It replaces three raw division expressions inside report_progress() that compute the on-screen progress percentage. The divisions are progress_substeps, progress_steps - progress_midpoint, and progress_steps. The change is defensive: it prevents a device fault/reset if any denominator is unexpectedly zero, but the denominators are normally set to non-zero values before progress reporting begins.
Changed components
legacy/firmware/signing.creport_progress() progress-bar computationInspect captured patch +13 / −6
diff --git a/legacy/firmware/signing.c b/legacy/firmware/signing.c
index 9646a827..f01d933a 100644
--- a/legacy/firmware/signing.c
+++ b/legacy/firmware/signing.c
@@ -381,6 +381,13 @@ static bool is_external_input(uint32_t i) {
return external_inputs[i / 32] & (1 << (i % 32));
}
+static uint32_t interpolate(uint32_t value, uint32_t num, uint32_t den) {
+ if (den == 0) {
+ return value;
+ }
+ return (value * num) / den;
+}
+
static void report_progress(bool force) {
static uint32_t update_ctr = 0;
if (!force && update_ctr < progress_update) {
@@ -392,17 +399,17 @@ static void report_progress(bool force) {
if (progress_midpoint != 0) {
if (progress_step < progress_midpoint) {
// Checking previous transactions.
- progress =
- (500 * progress_step + 500 * progress_substep / progress_substeps) /
- progress_midpoint;
+ progress = (500 * progress_step +
+ interpolate(500, progress_substep, progress_substeps)) /
+ progress_midpoint;
} else {
// Signing transaction after checking. No substeps.
- progress = 500 + 500 * (progress_step - progress_midpoint) /
- (progress_steps - progress_midpoint);
+ progress = 500 + interpolate(500, progress_step - progress_midpoint,
+ progress_steps - progress_midpoint);
}
} else {
// Loading transaction or signing transaction without checking. No substeps.
- progress = 1000 * progress_step / progress_steps;
+ progress = interpolate(1000, progress_step, progress_steps);
}
layoutProgress(progress_label, progress);
Why this scored 16/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.