fix(zcash): preflight the batch in the QR display path
What the developer wrote
Authored by Adam Tucker
fix(zcash): preflight the batch in the QR display path
The QR batch scan opens the Zcash batch view directly
(gui_scan_widgets.c), bypassing the model check step, so the preflight
that populates g_checkedBatch never ran before the view parsed for
display. Parsing then failed with "no checked Zcash batch available"
because the checked container was NULL.
Run the batch preflight from GuiParseZcashBatchData when g_checkedBatch
is missing (the USB path pre-checks via service_resolve_ur, so guard on
NULL to avoid a redundant second preflight), and surface a preflight
failure through the existing batch parse-error window.
What changed, and why it matters
This commit fixes a bug in the Keystone 3 hardware wallet's Zcash QR-code batch transaction flow. Previously, scanning a Zcash batch QR code skipped a required safety preflight check, causing the wallet to fail with a confusing 'no checked Zcash batch available' error instead of showing a meaningful error. The patch runs the missing preflight inside the QR display path and surfaces any failure message to the user. It is a reliability/usability fix rather than a demonstrated exploit.
Treat as a defensive hardening/bugfix. Review whether other QR-opened views (e.g., BTC multi-sig, other cypherpunk coins) similarly bypass model preflight checks. Ensure g_batchPreflightError is always zero-initialized and bounded (snprintf_s with sizeof is used correctly). No urgent patch rollout required unless the missing preflight is shown to enable a transaction-level attack.
Security signals we found
Missing input validation / preflight bypass in QR code display path
NULL-checked container used before population
Error-message surfacing improved to avoid generic failure dialogs
Potential for user confusion or denial-of-service via malformed Zcash batch QR (no exploit demonstrated)
Evidence from the diff
In gui_zcash_batch_widgets.c, the QR scan path opens the Zcash batch view directly without first running GuiGetZcashBatchCheckResult(), which populates g_checkedBatch. The parse step then dereferenced/relied on the NULL checked container and aborted. The change adds a g_batchPreflightError buffer, calls GuiGetZcashBatchCheckResult() inside GuiParseZcashBatchData when g_checkedBatch is NULL, copies any error_message into the buffer, and updates the parse-fail handler to prefer that message over a generic failure. USB mode already pre-checks via service_resolve_ur, so the NULL guard avoids double work there.
Changed components
src/ui/gui_widgets/multi/cypherpunk/gui_zcash_batch_widgets.cZcash batch QR display flowZcash batch transaction parsinggui_scan_widgets.c (caller, not modified)Inspect captured patch +36 / −4
diff --git a/src/ui/gui_widgets/multi/cypherpunk/gui_zcash_batch_widgets.c b/src/ui/gui_widgets/multi/cypherpunk/gui_zcash_batch_widgets.c
index 6416ed9..520ab11 100644
--- a/src/ui/gui_widgets/multi/cypherpunk/gui_zcash_batch_widgets.c
+++ b/src/ui/gui_widgets/multi/cypherpunk/gui_zcash_batch_widgets.c
@@ -33,6 +33,10 @@ static TransactionParseResult_DisplayZcashBatch *g_parseResult = NULL;
static DisplayZcashBatch *g_displayZcashBatch = NULL;
static DisplayPczt *g_currentTransaction = NULL;
static ZcashCheckedPczt *g_checkedBatch = NULL;
+// Holds a batch preflight failure message when the QR display path runs the
+// check itself (see GuiParseZcashBatchData), so the parse-fail handler can
+// surface it instead of a generic "invalid QR" window.
+static char g_batchPreflightError[128] = {0};
static PageWidget_t *g_pageWidget = NULL;
static lv_obj_t *g_cont = NULL;
@@ -375,6 +379,29 @@ void GuiZcashBatchWidgetsRefresh(void)
static void *GuiParseZcashBatchData(void)
{
+ g_batchPreflightError[0] = '\0';
+
+ // The QR scan path opens this view directly (gui_scan_widgets.c), bypassing
+ // the model check step that the USB path runs, so the preflight that
+ // populates g_checkedBatch has not run yet. g_checkedBatch is the normalized,
+ // checked bytes that this parse and later signing consume, so run the
+ // preflight here when it is missing. On failure, stash the real message for
+ // the parse-fail handler; only after it passes can parse succeed.
+ if (g_checkedBatch == NULL) {
+ PtrT_TransactionCheckResult checkResult = GuiGetZcashBatchCheckResult();
+ if (checkResult == NULL || checkResult->error_code != 0) {
+ if (checkResult != NULL) {
+ if (checkResult->error_message != NULL) {
+ snprintf_s(g_batchPreflightError, sizeof(g_batchPreflightError), "%s",
+ checkResult->error_message);
+ }
+ free_TransactionCheckResult(checkResult);
+ }
+ return NULL;
+ }
+ free_TransactionCheckResult(checkResult);
+ }
+
uint8_t sfp[32];
GetZcashSFP(GetCurrentAccountIndex(), sfp);
@@ -400,13 +427,18 @@ void GuiZcashBatchWidgetsTransactionParseSuccess(void)
void GuiZcashBatchWidgetsTransactionParseFail(void)
{
printf("GuiZcashBatchWidgetsTransactionParseFail\n");
- if (g_parseResult != NULL) {
- printf("error: %s\n", g_parseResult->error_message);
+ // A failed preflight leaves g_parseResult NULL but records its message in
+ // g_batchPreflightError; prefer whichever carries the real reason.
+ const char *errorMessage = (g_parseResult != NULL)
+ ? g_parseResult->error_message
+ : (g_batchPreflightError[0] != '\0' ? g_batchPreflightError : NULL);
+ if (errorMessage != NULL) {
+ printf("error: %s\n", errorMessage);
if (IsZcashBatchUsbMode()) {
- RespondZcashBatchUsbParseError(g_parseResult->error_message);
+ RespondZcashBatchUsbParseError(errorMessage);
return;
}
- g_parseErrorHintBox = GuiCreateZcashBatchParseErrorWindow(g_parseResult->error_message);
+ g_parseErrorHintBox = GuiCreateZcashBatchParseErrorWindow(errorMessage);
return;
}
if (IsZcashBatchUsbMode()) {
Why this scored 30/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.