fix(crypto): prevent calling `memzero(NULL, ...)`
What the developer wrote
Authored by Ondřej Vejpustek
fix(crypto): prevent calling `memzero(NULL, ...)`
[no changelog]
What changed, and why it matters
This commit fixes three places in the Trezor firmware's cryptographic code where a memory-clearing function could be called with a NULL pointer. In practice, passing NULL to memzero is harmless on Trezor's platform (it does nothing), but it is undefined behavior in C and could cause a crash or unpredictable results on other systems. The change adds simple NULL checks before clearing plaintext output and before wiping responder/initiator objects during cleanup.
Treat as a low-risk hardening fix. Review whether callers ever actually pass NULL to these functions; if not, this is defensive cleanup. No urgent security response is warranted based on the diff alone, but the patch should be merged for standards compliance and future portability.
Security signals we found
NULL pointer passed to memory-zeroing helper in cryptographic code
Undefined behavior in C standard library contract
Defensive hardening in Noise protocol implementation
No evidence of attacker-controlled NULL dereference path shown in diff
Evidence from the diff
The patch adds NULL guards before memzero() calls in crypto/noise_xxpsk3.c. Two guards are in deinit functions (noise_xxpsk3_responder_deinit and noise_xxpsk3_initiator_deinit), and one is in decrypt_with_ad when clearing plaintext after an AES-GCM authentication failure. The commit message states the purpose is to prevent calling memzero(NULL, …). The Trezor memzero implementation treats NULL as a no-op, so this is primarily a robustness/standards-compliance fix rather than an exploitable memory-safety bug on Trezor devices.
Changed components
crypto/noise_xxpsk3.cnoise_xxpsk3_responder_deinitnoise_xxpsk3_initiator_deinitdecrypt_with_adInspect captured patch +11 / −5
diff --git a/crypto/noise_xxpsk3.c b/crypto/noise_xxpsk3.c
index fd188ed2..b68bba69 100644
--- a/crypto/noise_xxpsk3.c
+++ b/crypto/noise_xxpsk3.c
@@ -287,7 +287,9 @@ static bool decrypt_with_ad(noise_xxpsk3_cipher_state_t *cs, const uint8_t *ad,
ciphertext + plaintext_len, NOISE_TAG_SIZE_BYTES,
&ctx) != RETURN_GOOD) {
memzero(&ctx, sizeof(ctx));
- memzero(plaintext, plaintext_len);
+ if (plaintext != NULL) {
+ memzero(plaintext, plaintext_len);
+ }
memzero(nonce_bytes, sizeof(nonce_bytes));
return false;
}
@@ -409,8 +411,10 @@ cleanup:
}
void noise_xxpsk3_responder_deinit(noise_xxpsk3_responder_t *rspn) {
- // Clear the responder structure
- memzero(rspn, sizeof(noise_xxpsk3_responder_t));
+ if (rspn != NULL) {
+ // Clear the responder structure
+ memzero(rspn, sizeof(noise_xxpsk3_responder_t));
+ }
}
bool noise_xxpsk3_responder_handle_request1(
@@ -634,8 +638,10 @@ cleanup:
}
void noise_xxpsk3_initiator_deinit(noise_xxpsk3_initiator_t *intr) {
- // Clear the initiator structure
- memzero(intr, sizeof(noise_xxpsk3_initiator_t));
+ if (intr != NULL) {
+ // Clear the initiator structure
+ memzero(intr, sizeof(noise_xxpsk3_initiator_t));
+ }
}
bool noise_xxpsk3_initiator_create_request1(
Why this scored 37/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.