Noticed while fixing #428(a) in passphrase_sm.c. Pre-existing, not introduced by that change.
The defect
bool passphrase_protect(void) {
bool ret = false;
PassphraseInfo passphrase_info; // <- stack, holds the plaintext
if (storage_getPassphraseProtected() && !session_isPassphraseCached()) {
if (passphrase_request(&passphrase_info)) {
session_cachePassphrase(passphrase_info.passphrase);
ret = true;
}
} else {
ret = true;
}
return (ret); // <- no memzero on any path
}wait_for_passphrase_ack() copies the host-supplied passphrase into passphrase_info->passphrase with strlcpy (passphrase_sm.c:67). passphrase_protect() then hands it to session_cachePassphrase() and returns, leaving the plaintext in its stack frame on every path — success, host cancel, and the early-out where passphrase protection is off.
grep -n "memzero\|CONFIDENTIAL" lib/firmware/passphrase_sm.c returns nothing: the buffer is neither zeroed nor placed in the CONFIDENTIAL section that the rest of the firmware uses for secret-bearing objects (compare confirm_sm.c:47's static CONFIDENTIAL char strbuf[...], and ethereum.c's static CONFIDENTIAL uint8_t privkey[32]).
Why it matters
The passphrase is a wallet-selecting secret: it is the difference between the visible wallet and a hidden one. Anything that later reads uninitialised stack in that region — a deep call path, an unrelated buffer over-read, a fault handler dumping stack — can surface it. It is the same class the firmware already defends against elsewhere by zeroing key material immediately after use.
Severity is bounded by needing another defect to read it, so this is defence-in-depth rather than a directly reachable disclosure.
Fix
memzero(&passphrase_info, sizeof(passphrase_info)) before every return in passphrase_protect(), and mark the type or the local CONFIDENTIAL so it lands in the section the rest of the firmware uses for secrets. Worth checking session_cachePassphrase()'s own copy discipline at the same time.
Not proposed for 7.14.2: it is untestable at the unit layer for the same reason #428(a) is, and it is unchanged from the shipped base.
Noticed while fixing #428(a) in
passphrase_sm.c. Pre-existing, not introduced by that change.The defect
wait_for_passphrase_ack()copies the host-supplied passphrase intopassphrase_info->passphrasewithstrlcpy(passphrase_sm.c:67).passphrase_protect()then hands it tosession_cachePassphrase()and returns, leaving the plaintext in its stack frame on every path — success, host cancel, and the early-out where passphrase protection is off.grep -n "memzero\|CONFIDENTIAL" lib/firmware/passphrase_sm.creturns nothing: the buffer is neither zeroed nor placed in theCONFIDENTIALsection that the rest of the firmware uses for secret-bearing objects (compareconfirm_sm.c:47'sstatic CONFIDENTIAL char strbuf[...], andethereum.c'sstatic CONFIDENTIAL uint8_t privkey[32]).Why it matters
The passphrase is a wallet-selecting secret: it is the difference between the visible wallet and a hidden one. Anything that later reads uninitialised stack in that region — a deep call path, an unrelated buffer over-read, a fault handler dumping stack — can surface it. It is the same class the firmware already defends against elsewhere by zeroing key material immediately after use.
Severity is bounded by needing another defect to read it, so this is defence-in-depth rather than a directly reachable disclosure.
Fix
memzero(&passphrase_info, sizeof(passphrase_info))before every return inpassphrase_protect(), and mark the type or the localCONFIDENTIALso it lands in the section the rest of the firmware uses for secrets. Worth checkingsession_cachePassphrase()'s own copy discipline at the same time.Not proposed for 7.14.2: it is untestable at the unit layer for the same reason #428(a) is, and it is unchanged from the shipped base.