feat(zcash): drive batch display/sign from the checked batch container
What the developer wrote
Authored by Adam Tucker
feat(zcash): drive batch display/sign from the checked batch container
What changed, and why it matters
This commit changes how a Zcash hardware wallet handles batches of transactions. Previously, the signing and display code used the raw, unchecked batch data directly. After the patch, the code first runs a checking function that produces a validated 'checked batch' object, stores it, and then uses only that validated object for parsing details shown to the user and for signing. This is a defensive refactor that reduces the risk of signing a transaction that differs from what was reviewed on screen, but the commit itself does not claim to fix a specific reported vulnerability.
Treat as a hardening/defensive fix. Review the implementation of check_zcash_batch_tx_cypherpunk() and the Rust PCZT checking logic to confirm the checked container is immutable and cannot be influenced by the raw UR data after checking. Verify that g_checkedBatch is always set before parse/sign and is freed on all error paths. No urgent user action is indicated by the commit alone.
Security signals we found
Separation of checked/sanitized transaction container from raw UR input
Display and signing paths now both consume the same checked object
Addition of explicit cleanup/free for the checked container
Function pointer casts made explicit
No mention of CVE, bug bounty, or external reporter in commit
Evidence from the diff
The patch modifies gui_zcash_batch_widgets.c so that GuiGetZcashBatchCheckResult() calls check_zcash_batch_tx_cypherpunk() with an output pointer to a new ZcashCheckedPczt object (g_checkedBatch). That checked object is then used as the input to parse_zcash_batch_tx_cypherpunk() (for on-screen display) and to SignZcashBatchInternal() (for producing the signature). The raw UR result data is no longer passed directly to parse or sign. A FreeCheckedBatch() helper is added and called during cleanup. The function-pointer casts for the signing callbacks are also made explicit. This aligns the displayed transaction data with the signed data, closing a potential class of ‘what you see is not what you sign’ (WYSIWYS) issues for Zcash batch transactions.
Changed components
src/ui/gui_widgets/multi/cypherpunk/gui_zcash_batch_widgets.cZcash batch transaction display flowZcash batch transaction signing flowCypherpunk edition Zcash UIInspect captured patch +21 / −12
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 c5a5ffd..6416ed9 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
@@ -32,6 +32,7 @@ static uint32_t g_txCount = 0;
static TransactionParseResult_DisplayZcashBatch *g_parseResult = NULL;
static DisplayZcashBatch *g_displayZcashBatch = NULL;
static DisplayPczt *g_currentTransaction = NULL;
+static ZcashCheckedPczt *g_checkedBatch = NULL;
static PageWidget_t *g_pageWidget = NULL;
static lv_obj_t *g_cont = NULL;
@@ -54,6 +55,14 @@ static bool IsZcashBatchUsbMode(void);
static void RejectZcashBatchUsbRequest(void);
static void RespondZcashBatchUsbParseError(const char *errorMessage);
+static void FreeCheckedBatch(void)
+{
+ if (g_checkedBatch != NULL) {
+ free_zcash_checked_pczt(g_checkedBatch);
+ g_checkedBatch = NULL;
+ }
+}
+
static void ClearPageData(void)
{
g_currentTxIndex = 0;
@@ -61,6 +70,8 @@ static void ClearPageData(void)
g_currentTransaction = NULL;
g_displayZcashBatch = NULL;
+ FreeCheckedBatch();
+
if (g_parseResult != NULL) {
free_TransactionParseResult_DisplayZcashBatch(g_parseResult);
g_parseResult = NULL;
@@ -82,14 +93,12 @@ void GuiSetZcashBatchUrData(URParseResult *urResult, URParseMultiResult *urMulti
UREncodeResult *GuiGetZcashBatchSignQrCodeData(void)
{
- void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
- return SignZcashBatchInternal(data, false);
+ return SignZcashBatchInternal(g_checkedBatch, false);
}
UREncodeResult *GuiGetZcashBatchSignUrDataUnlimited(void)
{
- void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
- return SignZcashBatchInternal(data, true);
+ return SignZcashBatchInternal(g_checkedBatch, true);
}
static bool IsZcashBatchUsbMode(void)
@@ -124,8 +133,8 @@ static UREncodeResult *SignZcashBatchInternal(void *data, bool unlimited)
return GuiSignZcashCypherpunkWithSeed(
data,
unlimited,
- sign_zcash_batch_tx_cypherpunk,
- sign_zcash_batch_tx_cypherpunk_unlimited);
+ (ZcashCypherpunkSignFunc)sign_zcash_batch_tx_cypherpunk,
+ (ZcashCypherpunkSignFunc)sign_zcash_batch_tx_cypherpunk_unlimited);
}
#ifdef CYPHERPUNK_VERSION
@@ -137,9 +146,11 @@ PtrT_TransactionCheckResult GuiGetZcashBatchCheckResult(void)
uint8_t accountNum = 0;
char ufvk[ZCASH_UFVK_MAX_LEN + 1] = {0};
+ FreeCheckedBatch();
+
GetExistAccountNum(&accountNum);
if (accountNum <= 0) {
- return check_zcash_batch_tx_cypherpunk(data, ufvk, sfp, zcashAccountIndex, true);
+ return check_zcash_batch_tx_cypherpunk(data, ufvk, sfp, zcashAccountIndex, true, &g_checkedBatch);
}
GetZcashSFP(GetCurrentAccountIndex(), sfp);
@@ -149,7 +160,8 @@ PtrT_TransactionCheckResult GuiGetZcashBatchCheckResult(void)
ufvk,
sfp,
zcashAccountIndex,
- !IsZcashSupportedForCurrentMnemonic());
+ !IsZcashSupportedForCurrentMnemonic(),
+ &g_checkedBatch);
}
#endif
@@ -363,18 +375,15 @@ void GuiZcashBatchWidgetsRefresh(void)
static void *GuiParseZcashBatchData(void)
{
- void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
uint8_t sfp[32];
- uint32_t zcashAccountIndex = 0;
GetZcashSFP(GetCurrentAccountIndex(), sfp);
char ufvk[ZCASH_UFVK_MAX_LEN + 1] = {0};
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
g_parseResult = parse_zcash_batch_tx_cypherpunk(
- data,
+ g_checkedBatch,
ufvk,
sfp,
- zcashAccountIndex,
!IsZcashSupportedForCurrentMnemonic());
return g_parseResult;
Why this scored 31/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.