main: fix error handling during PIN change
What the developer wrote
Authored by Mike Tolkachev
main: fix error handling during PIN change
What changed, and why it matters
This commit fixes a bug in the PIN-change flow on Blockstream Jade hardware wallets. Previously, if re-encrypting the wallet keys with a new PIN failed, or if the user abandoned the PIN change, the code did not jump to cleanup as intended. Instead, it could fall through and continue executing subsequent code paths that were meant to be skipped. This could lead to incorrect state handling, potential use of stale or invalid key material, or confusing behavior after a failed PIN change. The fix adds explicit 'goto cleanup' statements so the function exits the sensitive section safely in both error and abandonment cases.
Review the full PIN-change and key re-encryption flow for any additional missing cleanup or fall-through bugs. Verify that SENSITIVE_POP and sensitive-memory wiping occur consistently on all exit paths. Consider adding regression tests or static analysis checks for control-flow completeness in security-critical functions. Users should update firmware once a release containing this fix is available.
Security signals we found
Missing error-path control flow in cryptographic key handling
Potential use of stale or partially initialized key material after PIN-change failure
Cleanup bypass in sensitive authentication code path
PIN-change abandonment not properly terminated
Evidence from the diff
In main/process/auth_user.c, the function get_pin_load_keys() handles PIN entry and optional PIN change. After prompting for a new PIN and re-encrypting keys, two failure branches existed: (1) failure to re-encrypt with the new PIN, and (2) user abandonment of the PIN change. Both branches logged a warning/error and displayed an error message, but neither returned or jumped to cleanup. This caused fall-through into the SENSITIVE_POP(aeskey_new) call and subsequent code, even though the operation had failed. The patch adds ‘goto cleanup;’ after each await_error() in these branches, ensuring the function performs cleanup and exits without continuing into code paths that assume success. The change is small but corrects a control-flow bug in a security-critical authentication routine.
Changed components
main/process/auth_user.cget_pin_load_keys()PIN change / re-encryption flowInspect captured patch +2 / −0
diff --git a/main/process/auth_user.c b/main/process/auth_user.c
index 3e7cd24..2cdc7d5 100644
--- a/main/process/auth_user.c
+++ b/main/process/auth_user.c
@@ -269,10 +269,12 @@ static bool get_pin_load_keys(jade_process_t* process, const bool suppress_pin_c
} else {
JADE_LOGE("Failed to re-encrypt with changed PIN data");
await_error("Failed to re-encrypt key data!");
+ goto cleanup;
}
} else {
JADE_LOGW("Abandoned change-PIN");
await_error("Change-PIN abandoned");
+ goto cleanup;
}
SENSITIVE_POP(aeskey_new);
}
Why this scored 42/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.