What changed, and why it matters
This commit improves how the Keystone hardware wallet handles the secret Monero seed when creating key images and signing transactions. Previously, the seed was stored in a local variable inside a loop and its memory was cleared only if the operation succeeded. Now the seed buffer is declared outside the loop, its memory is always wiped with a secure zeroing function, and errors from fetching the seed are checked. This reduces the risk that the seed remains in memory after an error or after the operation finishes.
Treat as a defensive hardening improvement. Review whether other coin modules use similar patterns of conditional secret cleanup, and ensure `memset_s` is available and not optimized away by the toolchain. No immediate incident response is indicated by the diff alone.
Security signals we found
Sensitive material (Monero seed) previously left in stack memory on error paths
Clearing of secret cache was conditional on successful operation
Addition of secure zeroing with memset_s after use
Addition of error checking on GetAccountSeed return value
Commit title explicitly frames change as clearing seed memory
Evidence from the diff
The patch moves the uint8_t seed[64] buffer out of the do { ... } while(0) blocks in GuiGetMoneroKeyimagesQrCodeData and GuiGetMoneroSignedTransactionQrCodeData. It replaces a conditional length calculation with GetCurrentAccountSeedLen(), adds CHECK_ERRCODE_BREAK for the return value of GetAccountSeed, and moves ClearSecretCache() and a new memset_s(seed, sizeof(seed), 0, sizeof(seed)) call to run unconditionally after the loop. This ensures the seed buffer is zeroed and the secret cache is cleared even when GetAccountSeed or the Monero operation fails.
Changed components
src/ui/gui_chain/multi/cypherpunk/gui_monero.cMonero key image generation flowMonero transaction signing flowInspect captured patch +12 / −8
diff --git a/src/ui/gui_chain/multi/cypherpunk/gui_monero.c b/src/ui/gui_chain/multi/cypherpunk/gui_monero.c
index d2b0ebc..a75ec54 100644
--- a/src/ui/gui_chain/multi/cypherpunk/gui_monero.c
+++ b/src/ui/gui_chain/multi/cypherpunk/gui_monero.c
@@ -132,14 +132,16 @@ UREncodeResult *GuiGetMoneroKeyimagesQrCodeData(void)
SetLockScreen(false);
UREncodeResult *encodeResult;
void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
+ uint8_t seed[64];
do {
- uint8_t seed[64];
- int len = GetMnemonicType() == MNEMONIC_TYPE_BIP39 ? sizeof(seed) : GetCurrentAccountEntropyLen();
- GetAccountSeed(GetCurrentAccountIndex(), seed, SecretCacheGetPassword());
+ int len = GetCurrentAccountSeedLen();
+ int ret = GetAccountSeed(GetCurrentAccountIndex(), seed, SecretCacheGetPassword());
+ CHECK_ERRCODE_BREAK("GetAccountSeed", ret);
encodeResult = monero_generate_keyimage(data, seed, len, 0);
- ClearSecretCache();
CHECK_CHAIN_BREAK(encodeResult);
} while (0);
+ memset_s(seed, sizeof(seed), 0, sizeof(seed));
+ ClearSecretCache();
SetLockScreen(enable);
return encodeResult;
}
@@ -150,14 +152,16 @@ UREncodeResult *GuiGetMoneroSignedTransactionQrCodeData(void)
SetLockScreen(false);
UREncodeResult *encodeResult;
void *data = g_isMulti ? g_urMultiResult->data : g_urResult->data;
+ uint8_t seed[64];
do {
- uint8_t seed[64];
- int len = GetMnemonicType() == MNEMONIC_TYPE_BIP39 ? sizeof(seed) : GetCurrentAccountEntropyLen();
- GetAccountSeed(GetCurrentAccountIndex(), seed, SecretCacheGetPassword());
+ int len = GetCurrentAccountSeedLen();
+ int ret = GetAccountSeed(GetCurrentAccountIndex(), seed, SecretCacheGetPassword());
+ CHECK_ERRCODE_BREAK("GetAccountSeed", ret);
encodeResult = monero_generate_signature(data, seed, len, 0);
- ClearSecretCache();
CHECK_CHAIN_BREAK(encodeResult);
} while (0);
+ memset_s(seed, sizeof(seed), 0, sizeof(seed));
+ ClearSecretCache();
SetLockScreen(enable);
return encodeResult;
}
Why this scored 44/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.