What changed, and why it matters
This commit fixes a mismatch in how the firmware stores a recovery phrase (mnemonic) in memory. Previously, the code allocated memory based on the actual phrase length but then copied the phrase using a fixed maximum size. The change makes both steps use the same, correct length. This is a defensive fix that prevents potential memory corruption or undefined behavior when handling the secret recovery phrase.
Review the surrounding secret cache code for similar size mismatches, verify that SRAM_MALLOC failures are handled, and ensure that the mnemonic is always cleared from memory before freeing. Consider adding static analysis rules to catch mismatched allocation and copy sizes.
Security signals we found
Buffer size mismatch between allocation and copy operation
Use of safer string functions (strnlen_s, strcpy_s, memset_s)
Handling of high-value secret material (mnemonic/recovery phrase)
Memory allocation for sensitive data in SRAM
Evidence from the diff
In SecretCacheSetMnemonic(), the original code computed the mnemonic length with strnlen_s(mnemonic, MNEMONIC_MAX_LEN), allocated that many bytes plus one for the null terminator, but then called strcpy_s(g_mnemonicCache, MNEMONIC_MAX_LEN, mnemonic). The destination buffer size passed to strcpy_s (MNEMONIC_MAX_LEN) did not match the actual allocation, which could lead to a buffer overflow if the implementation of strcpy_s trusts the provided destination size, or to inconsistent behavior. The patch stores the computed length in a local variable and uses it consistently for both SRAM_MALLOC and strcpy_s, ensuring the destination size parameter matches the allocated buffer.
Changed components
src/crypto/secret_cache.cSecretCacheSetMnemonic()mnemonic cache storageInspect captured patch +3 / −2
diff --git a/src/crypto/secret_cache.c b/src/crypto/secret_cache.c
index 68ec8a6..eefa506 100644
--- a/src/crypto/secret_cache.c
+++ b/src/crypto/secret_cache.c
@@ -183,8 +183,9 @@ void SecretCacheSetMnemonic(char *mnemonic)
memset_s(g_mnemonicCache, MNEMONIC_MAX_LEN, 0, oldLen);
SRAM_FREE(g_mnemonicCache);
}
- g_mnemonicCache = SRAM_MALLOC(strnlen_s(mnemonic, MNEMONIC_MAX_LEN) + 1);
- strcpy_s(g_mnemonicCache, MNEMONIC_MAX_LEN, mnemonic);
+ size_t len = strnlen_s(mnemonic, MNEMONIC_MAX_LEN) + 1;
+ g_mnemonicCache = SRAM_MALLOC(len);
+ strcpy_s(g_mnemonicCache, len, mnemonic);
}
char *SecretCacheGetMnemonic(void)
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.