feat(zcash): store checked_PCZT in C and drive display/sign from it
What the developer wrote
Authored by Adam Tucker
feat(zcash): store checked_PCZT in C and drive display/sign from it
What changed, and why it matters
This commit refactors how the Keystone hardware wallet handles Zcash transaction data. Previously, the wallet parsed and signed directly from the raw incoming transaction data. Now it first runs a 'check' function that produces a validated intermediate object (checked_PCZT), stores that object, and uses it for both on-screen display and signing. The change also adds cleanup logic to free that stored object. This is a defensive code-quality improvement that reduces the chance the wallet will display one version of a transaction but sign a different one.
Review the Rust implementation of check_zcash_tx_multi_coins and check_zcash_tx_cypherpunk to confirm the checked_PCZT output is fully validated and that no path can return success while leaving g_checkedPczt uninitialized or partially initialized. Also verify that GuiGetZcashCheckResult is always called before display/signing and that FreeZcashMemory is invoked on UR result changes to prevent stale checked data being reused.
Security signals we found
Refactor to use a single validated intermediate representation for both display and signing
Addition of explicit cleanup/free path for the new checked_PCZT object
Function pointer casts added to align Rust FFI signers with C callback signatures
Potential TOCTOU or stale-data risk if g_checkedPczt is not freed/replaced atomically with the UR result it derives from
Evidence from the diff
The change introduces a static ZcashCheckedPczt pointer (g_checkedPczt) and a FreeCheckedPczt helper. GuiGetZcashCheckResult now calls the Rust check functions with an output pointer so they populate g_checkedPczt, and frees any prior value first. GuiGetZcashGUIData and the signing paths now use g_checkedPczt instead of re-deriving the raw UR data pointer. Function-pointer casts are added to match expected callback signatures. FreeZcashMemory now also frees the checked object. The commit does not change cryptographic logic; it centralizes the parsed/check result and ensures display and signing consume the same validated representation.
Changed components
src/ui/gui_chain/multi/gui_zcash.cZcash transaction display flowZcash transaction signing flowZcash PCZT checking/validation flowInspect captured patch +24 / −13
diff --git a/src/ui/gui_chain/multi/gui_zcash.c b/src/ui/gui_chain/multi/gui_zcash.c
index 4a362d0..9516bef 100644
--- a/src/ui/gui_chain/multi/gui_zcash.c
+++ b/src/ui/gui_chain/multi/gui_zcash.c
@@ -15,6 +15,15 @@ static URParseResult *g_urResult = NULL;
static URParseMultiResult *g_urMultiResult = NULL;
static void *g_parseResult = NULL;
static DisplayPczt *g_zcashData;
+static ZcashCheckedPczt *g_checkedPczt = NULL;
+
+static void FreeCheckedPczt(void)
+{
+ if (g_checkedPczt != NULL) {
+ free_zcash_checked_pczt(g_checkedPczt);
+ g_checkedPczt = NULL;
+ }
+}
#define CHECK_FREE_PARSE_RESULT(result) \
if (result != NULL) \
@@ -33,19 +42,21 @@ void GuiSetZcashUrData(URParseResult *urResult, URParseMultiResult *urMultiResul
void *GuiGetZcashGUIData(void)
{
CHECK_FREE_PARSE_RESULT(g_parseResult);
- void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
+ if (g_checkedPczt == NULL) {
+ return NULL;
+ }
uint8_t sfp[32];
GetZcashSFP(GetCurrentAccountIndex(), sfp);
PtrT_TransactionParseResult_DisplayPczt parseResult = NULL;
do {
#ifdef WEB3_VERSION
- parseResult = parse_zcash_tx_multi_coins(data, sfp);
+ parseResult = parse_zcash_tx_multi_coins(g_checkedPczt, sfp);
#endif
#ifdef CYPHERPUNK_VERSION
char ufvk[ZCASH_UFVK_MAX_LEN] = {'\0'};
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
- parseResult = parse_zcash_tx_cypherpunk(data, ufvk, sfp);
+ parseResult = parse_zcash_tx_cypherpunk(g_checkedPczt, ufvk, sfp);
#endif
CHECK_CHAIN_BREAK(parseResult);
g_zcashData = parseResult->data;
@@ -327,14 +338,15 @@ PtrT_TransactionCheckResult GuiGetZcashCheckResult(void)
MnemonicType mnemonicType = GetMnemonicType();
printf("mnemonicType: %d\n", mnemonicType);
+ FreeCheckedPczt();
#ifdef WEB3_VERSION
char *xpub = GetCurrentAccountPublicKey(XPUB_TYPE_ZEC_TRANSPARENT_LEGACY);
- return check_zcash_tx_multi_coins(data, xpub, sfp, zcash_account_index, mnemonicType == MNEMONIC_TYPE_SLIP39);
+ return check_zcash_tx_multi_coins(data, xpub, sfp, zcash_account_index, mnemonicType == MNEMONIC_TYPE_SLIP39, &g_checkedPczt);
#endif
#ifdef CYPHERPUNK_VERSION
char ufvk[ZCASH_UFVK_MAX_LEN + 1] = {0};
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
- return check_zcash_tx_cypherpunk(data, ufvk, sfp, zcash_account_index, !IsZcashSupportedForCurrentMnemonic());
+ return check_zcash_tx_cypherpunk(data, ufvk, sfp, zcash_account_index, !IsZcashSupportedForCurrentMnemonic(), &g_checkedPczt);
#endif
}
@@ -391,28 +403,26 @@ static UREncodeResult *SignZcashCypherpunkInternal(void *data, bool unlimited)
return GuiSignZcashCypherpunkWithSeed(
data,
unlimited,
- sign_zcash_tx_cypherpunk,
- sign_zcash_tx_cypherpunk_unlimited);
+ (ZcashCypherpunkSignFunc)sign_zcash_tx_cypherpunk,
+ (ZcashCypherpunkSignFunc)sign_zcash_tx_cypherpunk_unlimited);
}
#endif
UREncodeResult *GuiGetZcashSignQrCodeData(void)
{
- void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
#ifdef CYPHERPUNK_VERSION
- return SignZcashCypherpunkInternal(data, false);
+ return SignZcashCypherpunkInternal(g_checkedPczt, false);
#else
- return SignInternal(sign_zcash_tx, data);
+ return SignInternal((SignFn)sign_zcash_tx, g_checkedPczt);
#endif
}
UREncodeResult *GuiGetZcashSignUrDataUnlimited(void)
{
- void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
#ifdef CYPHERPUNK_VERSION
- return SignZcashCypherpunkInternal(data, true);
+ return SignZcashCypherpunkInternal(g_checkedPczt, true);
#else
- return SignInternal(sign_zcash_tx_unlimited, data);
+ return SignInternal((SignFn)sign_zcash_tx_unlimited, g_checkedPczt);
#endif
}
@@ -421,4 +431,5 @@ void FreeZcashMemory(void)
CHECK_FREE_UR_RESULT(g_urResult, false);
CHECK_FREE_UR_RESULT(g_urMultiResult, true);
CHECK_FREE_PARSE_RESULT(g_parseResult);
+ FreeCheckedPczt();
}
Why this scored 26/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.