fix(zcash): size UFVK buffers for terminators
What the developer wrote
Authored by Adam Tucker
fix(zcash): size UFVK buffers for terminators
What changed, and why it matters
This commit fixes a buffer sizing bug in the Keystone hardware wallet's Zcash support. The code previously treated the maximum string length and the buffer size as the same number, leaving no guaranteed room for the null terminator at the end of the string. In several places it also copied data using the maximum length rather than the actual buffer size. The patch adds a dedicated buffer size constant that includes space for the terminator and uses it consistently. If left unfixed, this could lead to truncated or missing terminator bytes, which in a security device can cause crashes, incorrect key handling, or in worst-case scenarios memory corruption that an attacker might exploit.
Treat this as a security-relevant memory-safety fix. Review whether any Zcash UFVK data could be exactly ZCASH_UFVK_MAX_LEN bytes, confirm strcpy_s implementations enforce terminator writing, and verify downstream consumers of GetZcashUFVK assume a null-terminated string. Consider adding static analysis rules to catch sizeof/strlen mismatches for bounded buffers.
Security signals we found
Off-by-one buffer sizing for null terminator in Zcash UFVK handling
Use of string-length constant instead of buffer-size constant in strcpy_s and memset_s calls
Inconsistent buffer declarations across account manager and GUI code
Potential missing null terminator on 384-byte UFVK string copies
Memory safety fix in firmware handling of cryptographic viewing keys
Evidence from the diff
The change introduces ZCASH_UFVK_BUFFER_SIZE = ZCASH_UFVK_MAX_LEN + 1 and replaces uses of ZCASH_UFVK_MAX_LEN (and the literal 384) with the new buffer-size constant when declaring char arrays and when calling strcpy_s/memset_s. The previous code declared g_zcashUFVKcache.ufvkCache as [ZCASH_UFVK_MAX_LEN + 1] but then passed ZCASH_UFVK_MAX_LEN to strcpy_s and memset_s, meaning the destination size argument did not reflect the full buffer. Several stack buffers were declared as [ZCASH_UFVK_MAX_LEN] or [384], one byte too small for a 384-character string plus null terminator. The patch corrects these off-by-one sizing issues.
Changed components
src/managers/account_manager.csrc/managers/account_manager.hsrc/ui/gui_chain/multi/gui_zcash.csrc/ui/gui_widgets/multi/cypherpunk/gui_connect_wallet_widgets.csrc/ui/gui_widgets/multi/cypherpunk/gui_zcash_batch_widgets.csrc/ui/gui_widgets/multi/gui_standard_receive_widgets.cInspect captured patch +14 / −13
diff --git a/src/managers/account_manager.c b/src/managers/account_manager.c
index fd31b73..726189a 100644
--- a/src/managers/account_manager.c
+++ b/src/managers/account_manager.c
@@ -652,7 +652,7 @@ static void SetZcashUFVK(uint8_t accountIndex, const char* ufvk)
{
ASSERT(accountIndex <= 2);
g_zcashUFVKcache.accountIndex = accountIndex;
- strcpy_s(g_zcashUFVKcache.ufvkCache, ZCASH_UFVK_MAX_LEN, ufvk);
+ strcpy_s(g_zcashUFVKcache.ufvkCache, sizeof(g_zcashUFVKcache.ufvkCache), ufvk);
}
static void SetZcashSFP(uint8_t accountIndex, const uint8_t* seedFingerprint)
@@ -665,7 +665,7 @@ static void SetZcashSFP(uint8_t accountIndex, const uint8_t* seedFingerprint)
static void ClearZcashUFVK()
{
- memset_s(g_zcashUFVKcache.ufvkCache, ZCASH_UFVK_MAX_LEN, '\0', ZCASH_UFVK_MAX_LEN);
+ memset_s(g_zcashUFVKcache.ufvkCache, sizeof(g_zcashUFVKcache.ufvkCache), '\0', sizeof(g_zcashUFVKcache.ufvkCache));
memset_s(g_zcashUFVKcache.seedFingerprint, 32, 0, 32);
}
@@ -673,7 +673,7 @@ int32_t GetZcashUFVK(uint8_t accountIndex, char* outUFVK)
{
ASSERT(accountIndex <= 2);
if (g_zcashUFVKcache.accountIndex == accountIndex) {
- strcpy_s(outUFVK, ZCASH_UFVK_MAX_LEN, g_zcashUFVKcache.ufvkCache);
+ strcpy_s(outUFVK, ZCASH_UFVK_BUFFER_SIZE, g_zcashUFVKcache.ufvkCache);
return SUCCESS_CODE;
}
return ERR_ZCASH_INVALID_ACCOUNT_INDEX;
@@ -771,8 +771,8 @@ int32_t SetupZcashCache(uint8_t accountIndex, const char* password)
return ret;
}
- char ufvk[ZCASH_UFVK_MAX_LEN] = {'\0'};
- strcpy_s(ufvk, ZCASH_UFVK_MAX_LEN, response->data);
+ char ufvk[ZCASH_UFVK_BUFFER_SIZE] = {'\0'};
+ strcpy_s(ufvk, sizeof(ufvk), response->data);
free_simple_response_c_char(response);
SetZcashUFVK(accountIndex, ufvk);
CLEAR_ARRAY(ufvk);
diff --git a/src/managers/account_manager.h b/src/managers/account_manager.h
index 97bb87b..4bb1354 100644
--- a/src/managers/account_manager.h
+++ b/src/managers/account_manager.h
@@ -8,6 +8,7 @@
#define WALLET_NAME_MAX_LEN 16
#define ZCASH_UFVK_MAX_LEN 384
+#define ZCASH_UFVK_BUFFER_SIZE (ZCASH_UFVK_MAX_LEN + 1)
typedef enum {
PASSCODE_TYPE_PIN,
@@ -48,7 +49,7 @@ typedef struct {
typedef struct {
uint8_t accountIndex;
- char ufvkCache[ZCASH_UFVK_MAX_LEN + 1];
+ char ufvkCache[ZCASH_UFVK_BUFFER_SIZE];
uint8_t seedFingerprint[32];
} ZcashUFVKCache_t;
diff --git a/src/ui/gui_chain/multi/gui_zcash.c b/src/ui/gui_chain/multi/gui_zcash.c
index 0ecea04..de9273b 100644
--- a/src/ui/gui_chain/multi/gui_zcash.c
+++ b/src/ui/gui_chain/multi/gui_zcash.c
@@ -54,7 +54,7 @@ void *GuiGetZcashGUIData(void)
parseResult = parse_zcash_tx_multi_coins(g_checkedPczt, sfp);
#endif
#ifdef CYPHERPUNK_VERSION
- char ufvk[ZCASH_UFVK_MAX_LEN] = {'\0'};
+ char ufvk[ZCASH_UFVK_BUFFER_SIZE] = {'\0'};
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
parseResult = parse_zcash_tx_cypherpunk(g_checkedPczt, ufvk, sfp);
#endif
@@ -344,7 +344,7 @@ PtrT_TransactionCheckResult GuiGetZcashCheckResult(void)
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};
+ char ufvk[ZCASH_UFVK_BUFFER_SIZE] = {0};
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
return check_zcash_tx_cypherpunk(data, ufvk, sfp, zcash_account_index, !IsZcashSupportedForCurrentMnemonic(), &g_checkedPczt);
#endif
diff --git a/src/ui/gui_widgets/multi/cypherpunk/gui_connect_wallet_widgets.c b/src/ui/gui_widgets/multi/cypherpunk/gui_connect_wallet_widgets.c
index f7e27a8..a2c9a2c 100644
--- a/src/ui/gui_widgets/multi/cypherpunk/gui_connect_wallet_widgets.c
+++ b/src/ui/gui_widgets/multi/cypherpunk/gui_connect_wallet_widgets.c
@@ -354,7 +354,7 @@ UREncodeResult *GuiGetZecData(void)
ZcashKey data[1];
keys->data = data;
keys->size = 1;
- char ufvk[384] = {'\0'};
+ char ufvk[ZCASH_UFVK_BUFFER_SIZE] = {'\0'};
uint8_t sfp[32];
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
GetZcashSFP(GetCurrentAccountIndex(), sfp);
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 7c9f348..f1b5e08 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
@@ -148,7 +148,7 @@ PtrT_TransactionCheckResult GuiGetZcashBatchCheckResult(void)
uint8_t sfp[32] = {0};
uint32_t zcashAccountIndex = 0;
uint8_t accountNum = 0;
- char ufvk[ZCASH_UFVK_MAX_LEN + 1] = {0};
+ char ufvk[ZCASH_UFVK_BUFFER_SIZE] = {0};
FreeCheckedBatch();
@@ -409,7 +409,7 @@ static void *GuiParseZcashBatchData(void)
uint8_t sfp[32];
GetZcashSFP(GetCurrentAccountIndex(), sfp);
- char ufvk[ZCASH_UFVK_MAX_LEN + 1] = {0};
+ char ufvk[ZCASH_UFVK_BUFFER_SIZE] = {0};
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
g_parseResult = parse_zcash_batch_tx_cypherpunk(
g_checkedBatch,
diff --git a/src/ui/gui_widgets/multi/gui_standard_receive_widgets.c b/src/ui/gui_widgets/multi/gui_standard_receive_widgets.c
index 62ab46e..50242cd 100644
--- a/src/ui/gui_widgets/multi/gui_standard_receive_widgets.c
+++ b/src/ui/gui_widgets/multi/gui_standard_receive_widgets.c
@@ -917,7 +917,7 @@ static void ModelGetAddress(uint32_t index, AddressDataItem_t *item)
#ifdef CYPHERPUNK_VERSION
if (g_chainCard == HOME_WALLET_CARD_ZEC) {
- char ufvk[ZCASH_UFVK_MAX_LEN] = {'\0'};
+ char ufvk[ZCASH_UFVK_BUFFER_SIZE] = {'\0'};
GetZcashUFVK(GetCurrentAccountIndex(), ufvk);
result = generate_zcash_default_address(ufvk);
@@ -1200,4 +1200,4 @@ static uint32_t* GetCosmosChainCurrentSelectIndex()
return NULL;
}
}
-#endif
\ No newline at end of file
+#endif
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.