otp: wipe and don't log the otp secret
What changed, and why it matters
This commit fixes two security hygiene issues in the handling of one-time password (OTP) secrets in Blockstream Jade. First, it stops writing the raw OTP secret into device logs when decoding fails. Second, it marks the temporary memory buffer holding the decoded secret as sensitive so it is wiped after use rather than left behind. The change also removes a redundant size check because another function already performs it. There is no evidence in the commit of an active exploit or a reported incident.
Treat this as a defensive security hardening fix. Users running firmware before this commit should upgrade, especially if device logs may be collected or shared. Review whether any persisted logs from earlier firmware contain OTP secrets. No immediate incident response is indicated by the commit itself.
Security signals we found
Removal of secret material from error/log messages
Use of SENSITIVE_PUSH/SENSITIVE_POP to clear a stack buffer containing a cryptographic secret
Reduction of redundant length check, consolidating validation in base32_to_bin()
No functional change to OTP generation; only to secret handling and logging
Evidence from the diff
In main/otpauth.c, prepare_md_ctx() previously logged the full otp_ctx->secret value via JADE_LOGE on two error paths, including a sanity-check path that has now been removed. The secret is base32-encoded user input, but logging it to device logs increases exposure if logs are extracted. The patch removes the redundant SECRET_BUFSIZE sanity check (delegating length enforcement to base32_to_bin()) and restructures the function to use a single exit label. It wraps the secret_bin stack buffer with SENSITIVE_PUSH/SENSITIVE_POP so the buffer is zeroed before the function returns, reducing the window for secret material to remain in stack memory. The log messages are changed to omit the secret string.
Changed components
main/otpauth.cprepare_md_ctx()OTP secret decoding and HMAC setup pathInspect captured patch +8 / −9
diff --git a/main/otpauth.c b/main/otpauth.c
index f995407..64bb0e7 100644
--- a/main/otpauth.c
+++ b/main/otpauth.c
@@ -298,17 +298,13 @@ static bool prepare_md_ctx(const otpauth_ctx_t* otp_ctx, mbedtls_md_context_t* m
mbedtls_md_type_t md_type = get_md_type(otp_ctx);
OTP_CHECK_BOOL_RETURN(mbedtls_md_setup(md_ctx, mbedtls_md_info_from_type(md_type), 1) == 0);
- // Sanity check - can't really happen atm as entire URI length is limited
- if (otp_ctx->secret_len / 1.6 > SECRET_BUFSIZE) {
- JADE_LOGE("Bad Base32 secret decode - secret length: %.*s", otp_ctx->secret_len, otp_ctx->secret);
- return false;
- }
-
+ bool ret = false;
uint8_t secret_bin[SECRET_BUFSIZE];
+ SENSITIVE_PUSH(secret_bin, sizeof(secret_bin));
size_t secret_bin_len = base32_to_bin(otp_ctx->secret, otp_ctx->secret_len, secret_bin, sizeof(secret_bin));
if (!secret_bin_len) {
- JADE_LOGE("Bad Base32 secret decode - secret: %.*s", otp_ctx->secret_len, otp_ctx->secret);
- return false;
+ JADE_LOGE("Bad Base32 secret decode");
+ goto done;
}
// Do not lengthen/pad the secret for SHA1 *only* - for gauth compatibility.
@@ -320,8 +316,11 @@ static bool prepare_md_ctx(const otpauth_ctx_t* otp_ctx, mbedtls_md_context_t* m
const size_t hmac_size = mbedtls_md_get_size(md_ctx->MBEDTLS_PRIVATE(md_info));
pad_secret(secret_bin, &secret_bin_len, hmac_size);
}
+ ret = mbedtls_md_hmac_starts(md_ctx, secret_bin, secret_bin_len) == 0;
- return mbedtls_md_hmac_starts(md_ctx, secret_bin, secret_bin_len) == 0;
+done:
+ SENSITIVE_POP(secret_bin);
+ return ret;
}
bool otp_get_auth_code(const otpauth_ctx_t* otp_ctx, char* token, const size_t token_len)
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.