usb: simplify handle_ota_reply logic
What the developer wrote
Authored by Jon Griffiths
usb: simplify handle_ota_reply logic
What changed, and why it matters
This commit simplifies how a Blockstream Jade hardware wallet handles an over-the-air (OTA) firmware update reply received over USB. Previously, the code checked whether a CBOR boolean 'result' field existed and only updated the success flag if it did. Now it updates the success flag regardless of whether the field is present, and no longer treats a missing 'result' as a failure. This could mean a malformed or unexpected reply leaves the success flag set to its previous value rather than being explicitly cleared, potentially making an unsuccessful update appear successful.
Review the caller of handle_ota_reply() to confirm it initializes *ok to false before invocation. Consider restoring explicit clearing of *ok on CBOR parse failure or missing 'result' field to prevent a stale success flag. If this change was intentional, document the invariant that *ok must be pre-initialized.
Security signals we found
Removal of explicit failure path for missing 'result' field
Success flag (*ok) no longer explicitly set to false on parse or field errors
Function always returns true, relying solely on *ok to indicate outcome
Potential use of uninitialized or stale success flag in caller
Evidence from the diff
In main/usbhmsc/usbmode.c, handle_ota_reply() was refactored. The original code initialized a local bool_result to false, required rpc_get_boolean(‘result’, …) to succeed, and only then wrote the value to ok. The new code calls rpc_get_boolean(‘result’, &message, ok) directly inside an else branch after CBOR validation. If the CBOR is invalid or the ‘result’ field is missing, ok is never modified. Because the caller’s ok is not initialized in this diff and the function always returns true to stop waiting, a stale or uninitialized ok could be interpreted as success. This is a logic change that weakens error handling, though it is not a clear-cut exploit on its own.
Changed components
main/usbhmsc/usbmode.chandle_ota_reply()OTA firmware update over USBInspect captured patch +2 / −9
diff --git a/main/usbhmsc/usbmode.c b/main/usbhmsc/usbmode.c
index e7dfb55..804e371 100644
--- a/main/usbhmsc/usbmode.c
+++ b/main/usbhmsc/usbmode.c
@@ -566,17 +566,10 @@ static bool handle_ota_reply(const uint8_t* msg, const size_t len, void* ctx)
const CborError cberr = cbor_parser_init(msg, len, CborValidateBasic, &parser, &message);
if (cberr != CborNoError || !rpc_message_valid(&message)) {
JADE_LOGE("Invalid cbor message");
- goto cleanup;
- }
-
- bool bool_result = false;
- if (!rpc_get_boolean("result", &message, &bool_result)) {
- goto cleanup;
+ } else {
+ rpc_get_boolean("result", &message, ok);
}
- *ok = bool_result;
-
-cleanup:
// We return true in all cases to indicate that a message was received
// and we should stop waiting - whether the message was processed 'successfully'
// is indicated by the 'ok' flag in the passed context object.
Why this scored 34/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.