fix(crypto): add missing memzero to `bip32.c`
What the developer wrote
Authored by M1nd3r
fix(crypto): add missing memzero to `bip32.c`
[no changelog]
What changed, and why it matters
This commit fixes a cleanup oversight in the Trezor firmware's code that handles NEM cryptocurrency encryption and decryption. Previously, if an encryption or decryption operation failed partway through, the temporary AES key context (a small chunk of memory holding sensitive key material) was not securely wiped before the function returned. The patch ensures this memory is always zeroed out, even on error paths, reducing the risk that leftover cryptographic key bits could leak to other code or be recovered later. It is a defensive hardening fix rather than a demonstrated exploitable bug.
Treat as a low-to-moderate security hardening fix. Users running NEM operations on affected firmware should update when convenient. Developers should audit other crypto functions for similar missing `memzero` calls on error paths and consider static analysis rules that enforce cleanup before every return.
Security signals we found
Missing secure zeroization (memzero) on error paths
Sensitive cryptographic context (AES key schedule) left on stack after failure
NEM-specific BIP32 encrypt/decrypt functions affected
Defensive fix with no changelog entry
Evidence from the diff
In crypto/bip32.c, the functions hdnode_nem_encrypt and hdnode_nem_decrypt allocate stack-based aes_encrypt_ctx / aes_decrypt_ctx structures and initialize them from a derived shared_key. The existing code already used memzero(shared_key, sizeof(shared_key)) after key schedule setup, but on the early-return error paths (aes_encrypt_key256, aes_cbc_encrypt, aes_cbc_decrypt failures) the AES context itself was not cleared. Because these contexts can retain expanded key material and internal state, the patch introduces a single cleanup exit path and calls memzero(&ctx, sizeof(ctx)) before returning. This is a classic secure-cleanup fix for a use-after-free-style information disclosure via stale stack data, though no specific exploit is described in the commit materials.
Changed components
crypto/bip32.chdnode_nem_encrypthdnode_nem_decryptInspect captured patch +15 / −7
diff --git a/crypto/bip32.c b/crypto/bip32.c
index 3ebd1aaa..7bbaa7d5 100644
--- a/crypto/bip32.c
+++ b/crypto/bip32.c
@@ -566,23 +566,27 @@ int hdnode_nem_encrypt(const HDNode *node, const ed25519_public_key public_key,
aes_encrypt_ctx ctx = {0};
+ int succ = 0;
int ret = aes_encrypt_key256(shared_key, &ctx);
memzero(shared_key, sizeof(shared_key));
if (ret != EXIT_SUCCESS) {
- return 0;
+ goto cleanup;
}
if (aes_cbc_encrypt(payload, buffer, size, iv, &ctx) != EXIT_SUCCESS) {
- return 0;
+ goto cleanup;
}
if (aes_cbc_encrypt(last_block, &buffer[size], sizeof(last_block), iv,
&ctx) != EXIT_SUCCESS) {
- return 0;
+ goto cleanup;
}
- return 1;
+ succ = 1;
+cleanup:
+ memzero(&ctx, sizeof(ctx));
+ return succ;
}
int hdnode_nem_decrypt(const HDNode *node, const ed25519_public_key public_key,
@@ -596,18 +600,22 @@ int hdnode_nem_decrypt(const HDNode *node, const ed25519_public_key public_key,
aes_decrypt_ctx ctx = {0};
+ int succ = 0;
int ret = aes_decrypt_key256(shared_key, &ctx);
memzero(shared_key, sizeof(shared_key));
if (ret != EXIT_SUCCESS) {
- return 0;
+ goto cleanup;
}
if (aes_cbc_decrypt(payload, buffer, size, iv, &ctx) != EXIT_SUCCESS) {
- return 0;
+ goto cleanup;
}
- return 1;
+ succ = 1;
+cleanup:
+ memzero(&ctx, sizeof(ctx));
+ return succ;
}
#endif
Why this scored 48/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.