fix(zcash): error handling in setup functions
What the developer wrote
Authored by John Boyd
fix(zcash): error handling in setup functions
- Check return values of SetupZcashCache/SetupZcashSFP and
AccountPublicInfoSwitch in CreateNewAccount and VerifyPasswordAndLogin
- Fix seed buffer leaks in SetupZcashSFP and SetupZcashCache error paths
- Fix wrong variable (response -> responseSFP) in SetupZcashCache
SFP error path
- Free response objects before returning on error
- Clear UFVK buffer after use
- Check for NULL encrypted UFVK before decryption
Co-authored-by: Daira-Emma Hopwood <daira@jacaranda.org>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
What changed, and why it matters
This commit fixes several bugs in the Zcash wallet setup code for a hardware wallet. Previously, the code could ignore errors when setting up Zcash data, leak sensitive seed material in memory, use the wrong error variable, and try to decrypt a missing encrypted key. These are reliability and security hygiene issues rather than a single obvious remote exploit, but they could let a device finish account creation or login even though Zcash keys were not set up correctly, and could leave secret seed bytes in memory longer than intended.
Treat this as a security-hardening fix and include it in the next firmware release. Review other Rust FFI response wrappers for similar missing free/clear patterns, and consider adding static analysis rules to enforce return-value checks and secure buffer clearing for seed/UFVK handling.
Security signals we found
Unchecked return values in security-critical setup functions
Sensitive buffer (seed) not cleared on error paths
Use of wrong variable in error-handling path (response vs responseSFP)
Missing NULL check before decrypting encrypted UFVK
Memory/object leaks of response structs on error paths
UFVK plaintext left in stack buffer after use
Evidence from the diff
The patch hardens Zcash setup paths in account_manager.c. It adds return-value checks for AccountPublicInfoSwitch, SetupZcashCache, and SetupZcashSFP in CreateNewAccount and VerifyPasswordAndLogin so failures no longer pass silently. It clears the seed buffer on every error path in SetupZcashSFP and SetupZcashCache, fixes a bug where SetupZcashCache’s SFP error path referenced response instead of responseSFP, frees SimpleResponse objects before returning on errors, clears the UFVK buffer after use, and adds a NULL check before decrypting the encrypted UFVK. These are defensive fixes for memory hygiene and error propagation in code that handles highly sensitive key material.
Changed components
src/managers/account_manager.cCreateNewAccountVerifyPasswordAndLoginSetupZcashSFPSetupZcashCacheInspect captured patch +47 / −7
diff --git a/src/managers/account_manager.c b/src/managers/account_manager.c
index 6b09657..db5f150 100644
--- a/src/managers/account_manager.c
+++ b/src/managers/account_manager.c
@@ -139,13 +139,15 @@ int32_t CreateNewAccount(uint8_t accountIndex, const uint8_t *entropy, uint8_t e
ret = SaveCurrentAccountInfo();
CHECK_ERRCODE_RETURN_INT(ret);
ret = AccountPublicInfoSwitch(g_currentAccountIndex, password, true);
+ CHECK_ERRCODE_RETURN_INT(ret);
#ifdef CYPHERPUNK_VERSION
- SetupZcashCache(accountIndex, password);
+ ret = SetupZcashCache(accountIndex, password);
+ CHECK_ERRCODE_RETURN_INT(ret);
#endif
#ifdef WEB3_VERSION
- SetupZcashSFP(accountIndex, password);
-#endif
+ ret = SetupZcashSFP(accountIndex, password);
CHECK_ERRCODE_RETURN_INT(ret);
+#endif
return ret;
}
@@ -247,11 +249,14 @@ int32_t VerifyPasswordAndLogin(uint8_t *accountIndex, const char *password)
printf("passphrase not exist, info switch\r\n");
ret = AccountPublicInfoSwitch(g_currentAccountIndex, password, false);
}
+ CHECK_ERRCODE_RETURN_INT(ret);
#ifdef CYPHERPUNK_VERSION
- SetupZcashCache(*accountIndex, password);
+ ret = SetupZcashCache(*accountIndex, password);
+ CHECK_ERRCODE_RETURN_INT(ret);
#endif
#ifdef WEB3_VERSION
- SetupZcashSFP(*accountIndex, password);
+ ret = SetupZcashSFP(*accountIndex, password);
+ CHECK_ERRCODE_RETURN_INT(ret);
#endif
} else {
g_publicInfo.loginPasswordErrorCount++;
@@ -625,10 +630,17 @@ int32_t SetupZcashSFP(uint8_t accountIndex, const char* password)
uint8_t seed[SEED_LEN];
int len = GetMnemonicType() == MNEMONIC_TYPE_BIP39 ? sizeof(seed) : GetCurrentAccountEntropyLen();
int32_t ret = GetAccountSeed(accountIndex, seed, password);
+ if (ret != SUCCESS_CODE) {
+ CLEAR_ARRAY(seed);
+ return ret;
+ }
+
SimpleResponse_u8 *responseSFP = calculate_zcash_seed_fingerprint(seed, len);
+ CLEAR_ARRAY(seed);
if (responseSFP->error_code != 0) {
ret = responseSFP->error_code;
printf("error: %s\r\n", responseSFP->error_message);
+ free_simple_response_u8(responseSFP);
return ret;
}
@@ -654,25 +666,53 @@ int32_t SetupZcashCache(uint8_t accountIndex, const char* password)
uint8_t seed[SEED_LEN];
int len = GetMnemonicType() == MNEMONIC_TYPE_BIP39 ? sizeof(seed) : GetCurrentAccountEntropyLen();
int32_t ret = GetAccountSeed(accountIndex, seed, password);
+ if (ret != SUCCESS_CODE) {
+ CLEAR_ARRAY(seed);
+ return ret;
+ }
SimpleResponse_u8 *iv_response = rust_derive_iv_from_seed(seed, len);
+ if (iv_response->error_code != 0) {
+ ret = iv_response->error_code;
+ CLEAR_ARRAY(seed);
+ printf("error: %s\r\n", iv_response->error_message);
+ free_simple_response_u8(iv_response);
+ return ret;
+ }
uint8_t iv_bytes[16];
memcpy_s(iv_bytes, 16, iv_response->data, 16);
free_simple_response_u8(iv_response);
char *zcashEncrypted = GetCurrentAccountPublicKey(ZCASH_UFVK_ENCRYPTED_0);
+ if (zcashEncrypted == NULL) {
+ CLEAR_ARRAY(seed);
+ CLEAR_ARRAY(iv_bytes);
+ return ERR_GENERAL_FAIL;
+ }
+
SimpleResponse_c_char *response = rust_aes256_cbc_decrypt(zcashEncrypted, password, iv_bytes, 16);
+ CLEAR_ARRAY(iv_bytes);
+ if (response->error_code != 0) {
+ ret = response->error_code;
+ CLEAR_ARRAY(seed);
+ printf("error: %s\r\n", response->error_message);
+ free_simple_response_c_char(response);
+ return ret;
+ }
char ufvk[ZCASH_UFVK_MAX_LEN] = {'\0'};
strcpy_s(ufvk, ZCASH_UFVK_MAX_LEN, response->data);
free_simple_response_c_char(response);
SetZcashUFVK(accountIndex, ufvk);
+ CLEAR_ARRAY(ufvk);
SimpleResponse_u8 *responseSFP = calculate_zcash_seed_fingerprint(seed, len);
+ CLEAR_ARRAY(seed);
if (responseSFP->error_code != 0) {
- ret = response->error_code;
- printf("error: %s\r\n", response->error_message);
+ ret = responseSFP->error_code;
+ printf("error: %s\r\n", responseSFP->error_message);
+ free_simple_response_u8(responseSFP);
return ret;
}
Why this scored 59/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.