release: review 7.15 after current 7.14.3 - #628
Merged
Conversation
Cut from 1af2ffe, which is the shipped v7.14.1 plus 12 EVM bugfix commits and the head of develop on both this fork and upstream. Bumps VERSION to 7.14.2 and records the release plan: base selection and why the old release/7.14.2 branch (199e42a, preserved as abandoned/7.14.2-20260515) was not it, the threat model, the fourteen tracked defects, and the evidence standard for the memory-safety claims. No functional change. Fixes land on this branch as separate commits.
7.14.2 adds no features, so it needs nothing outside master on either repo. Both previous pins are strict ancestors of master (ahead=0, behind=1); the single commit each master carries beyond them is an identical CI-only addition (.github/workflows/copilot-review.yml, +19/-0). Repinning is therefore a superset move with no effect on generated code. Verified device-protocol master still defines every message this tree handles: TonSignTx, TronSignTx, SolanaSignTx/SolanaTokenInfo, the EthereumSignTx family, OsmosisMsgSend and CosmosMsgSend, with their .options bounds. deps/crypto/trezor-firmware is deliberately NOT repinned: its canonical branch is 'keepkey', not master, and .gitmodules declares no branch for it.
Guard the four discarded confirm() returns in fsm_msgEthereumSignTypedHash, and add one real confirm() gate immediately before eip712_sign() in e712_types_values, so no EIP-712 signature can be produced without a physical button hold.
Risk: Behaviour changes a legitimate host/user will notice:
1. EthereumSignTypedHash: no visual change in the happy path — the same four screens, each still advanced by the same button hold. Only difference: a host that sends Cancel (or Initialize) during any screen now gets Failure_ActionCancelled instead of a signature. Hosts that never cancel see no change.
2. Ethereum712TypesValues: ONE new OLED screen at the end of the message (eip712typevals != 1) call — title "Sign Typed Data", body "Sign with address 0x<checksummed 40 hex>?" (~62 chars, well under BODY_CHAR_MAX=352 and shorter than the existing "Confirm hash digest: <64 hex>" screen that already ships). One extra button hold per typed-data signature. The domain call (eip712typevals == 1) is untouched — no new screen, it signs nothing.
3. If the user does not confirm that new screen, have_ds is cleared, so a host retrying the message call gets MSG_NO_DS ("EIP-712 domain seperator hash must be calculated first") and must re-send the domain message. Any host that implements retry-after-cancel without re-sending the domain will now see that failure instead of a second prompt. This matches what already happens after a successful signature.
4. Existing pyk tests that drive Ethereum712TypesValues will now see one additional ButtonRequest on the message call; tests using an auto-ack debuglink client are unaffected, tests that count ButtonRequests will need +1.
Residual (NOT closed by this patch, deliberately): the per-field screens inside eip712.c remain (void)review(...), so the user still cannot abort mid-review and the individual field values are still display-only. Closing that requires the confirm-propagation rework (new USER_CANCELLED code, dsConfirm() returning int, 9 checked call sites in parseVals) that 7.15 did on top of confirm_bytes; it is real new machinery and out of scope for 7.14.2. The security defect — signature with zero button presses — is closed by the terminal gate.
Also unchanged by design: no AdvancedMode policy gate on the raw-hash endpoint (that is 7.15's ethereum_typed_hash_policy_allows and would block a currently-working host path).
Closes #403
…n table
Extend the raw-calldata disclosure guard in `ethereum_signing_init` so the `UnknownToken` sentinel (returned on a token-table miss) is treated like `NULL`, routing untabled ERC-20 calls through the warning + `confirm("Confirm Ethereum Data")` gate.
Risk: Behaviour change is confined to `data_total == 68` + standard ERC-20 transfer/approve selector + `tokenByChainAddress()` miss + `data_needs_confirm` (i.e. not a `ethereum_contractHandled` path).
What a user will notice on the OLED for an untabled token:
- One extra screen: "Confirm Ethereum Data" showing the first 24 calldata bytes (`a9059cbb` + zero padding + the leading 8 bytes of the recipient) and "68 bytes", requiring a physical button hold.
- Plus, when the AdvancedMode policy is off, the existing "Warning / Signing of arbitrary ETH contract data..." review screen.
- The preceding "Send Unknown token value to 0x<recipient>" / "Approve withdrawal of up to Unknown token value by 0x..." screen is unchanged, and the fee screen is unchanged (`token != NULL` is still true, so it still says "the tokens").
Hosts sending a tabled token see zero difference. Non-ERC-20 contract calls see zero difference. `ethereum_getStandardERC20Coin` (:107) and `ethereumFormatAmount` (:337) are untouched, so the TRANSFER/coin-lookup paths are unaffected.
The one real friction: a legitimate host that sends a genuinely-new token (not yet in `ethereum_tokens.def`) now costs the user two extra button holds. That is the intended trade — the device was previously signing calldata it never showed.
Explicitly NOT fixed here (flag for a separate issue): the AdvancedMode warning at :770 is still `(void)review(...)`, i.e. informational and not a gate; and neither the ERC-20 confirm screen nor the data screen ever shows the contract address (`msg->to`) for an untabled token. #412 is about reaching the disclosure; both of those are distinct defects with wider blast radius.
Closes #412
Stop rendering the unbindable to_address/amount in fsm_msgTonSignTx; replace both confirms with a single honest blind-sign screen naming the raw_tx byte count, behind an AdvancedMode policy gate. Risk: Behaviour changes a legitimate host will notice: 1. TON signing now requires AdvancedMode. With AdvancedMode off (and per prior hardware findings it is session state — it is OFF after every power cycle), TonSignTx returns Failure_ActionCancelled "Transaction signing disabled by policy" after a "Blocked" review screen. Every existing TON host flow breaks until the user enables it. This is a real, deliberate availability regression; see decision_note. 2. The ButtonRequest count drops from two to one. BASE emitted ButtonRequest_ConfirmOutput (only when has_to_address && has_amount) followed by ButtonRequest_SignTx. Now only ButtonRequest_SignTx is emitted, plus one ButtonRequest_Other on the blocked path. Hosts that drive the ButtonRequest/ButtonAck loop generically (hdwallet, pioneer) are fine; anything that hardcodes "expect exactly two button requests for TON" would desync. 3. OLED text changes: title "Transaction" -> "TON Blind Sign", body "Really sign this TON transaction?" -> "Sign N-byte TON transaction?", and the "Send X TON to <addr>?" screen disappears entirely. Users will read this as a downgrade in information; it is in fact the removal of information the device was never able to verify. With the current proto (raw_tx = 32-byte body cell hash) the normal reading is "Sign 32-byte TON transaction?". 4. ton_formatAmount() (lib/firmware/ton.c:221, declared include/keepkey/firmware/ton.h:58) loses its only caller and becomes dead code. It is a non-static extern function so this produces no warning and no build break; 7.15 left it in place too. I deliberately did not delete it — leaving it costs a few bytes of flash and keeps the diff to a single file. Not a risk: no change to the bytes that get signed, no proto/.options change, no new symbol, no new header. The private key is still memzero'd on every exit path, and the new blocked path returns before fsm_getDerivedNode so no key material is ever derived on it. Closes #404
…ruction Scale SPL TransferChecked amounts only by the decimals inside the signed instruction, show the authenticated mint on its own screen, and drop the host symbol when its claimed decimals disagree or the text is not safe printable ASCII. Risk: Behaviour changes a legitimate host will notice: 1. ONE EXTRA SCREEN per SPL TransferChecked instruction — a \"Token mint\" confirm carrying the full base58 mint, requiring an additional button hold before the amount screen. Plain Transfer (ix 3) is unaffected because has_mint is false there. Multi-instruction token transactions gain one screen per checked transfer (capped at SOL_MAX_INSTRUCTIONS = 8). 2. The unlabelled fallback now reads e.g. \"1.000000 tokens\" instead of \"1000000 tokens\", because it scales by the signed decimals. Strictly more informative, but the string on the OLED changes; any screenshot/OLED golden covering an SPL transfer will need re-baselining. 3. A host that sends token_info whose decimals disagree with the signed instruction silently loses its symbol (shows \"tokens\"). No host with correct metadata is affected. Vault/Pioneer should be spot-checked for hardcoded or stale decimals in the SolanaSignTx builder — if it fills decimals from a token list rather than from the instruction it built, users would see \"tokens\" where they used to see a symbol. 4. A TransferChecked whose data field is 9 bytes now classifies as unknown, making the whole transaction OPAQUE and therefore blocked unless AdvancedMode is on. No conforming host emits that, but a host that hand-rolls the instruction and drops the trailing decimals byte would go from \"signs cleanly\" to \"Enable AdvancedMode to blind-sign\". Not closed by this patch, by design: a host can still name a worthless mint \"USDC\" with matching decimals. BASE has no on-device token table and no signed-metadata verifier, so the symbol is fundamentally unverifiable here. The mitigation is that the authenticated mint is now always disclosed on its own screen, so the symbol is decoration rather than the identity. Full closure is the 7.15 solana_token_info_trusted / known-token work and is explicitly out of scope. ROM/RAM: negligible — one 45-byte stack buffer in an existing case block, one small static predicate; the removed snprintf path roughly offsets it. Closes #406
In fsm_msgCosmosMsgAck's IBC branch, require has_receiver/has_amount and confirm the actual signed receiver (and sender) with confirm_cosmos_address instead of printing the sender as the destination.
Risk: Behaviour change a legitimate host/user will notice: the IBC branch goes from 5 confirm screens to 7. The amount screen no longer names an address at all ("Transfer 1.234 ATOM via IBC?" instead of "Transfer 1.234 ATOM to cosmos1...?"), and two new address screens appear before the source-channel screen: "Confirm sender address" then "Confirm dest. address", each on the cosmos address layout (large address + QR), each requiring a button hold. Two extra holds per IBC transfer.
Deliberate choice worth reviewing: I kept a sender screen. The sender is information the user has today (mislabelled), the firmware never checks it against the device key, and ORPHAN also shows it -- so dropping it would be a silent disclosure regression shipped inside a security fix. If the reviewer prefers strictly two extra-screens-minus-one, delete the "Confirm sender address" block; the receiver block alone still closes #407.
Newly-rejected host messages: an IBC message omitting `receiver` or `amount` now fails fast with Failure_FirmwareError "Message is missing required parameters". Omitting `receiver` already failed at BASE (bech32_decode("") returns false -> Failure_SyntaxError), just later and after a wasted button hold; omitting `amount` previously succeeded and signed a 0-amount transfer. Any host relying on amount-defaults-to-0 breaks -- that is intended.
Compile surface checked at BASE: msg->ibc_transfer.has_receiver / .has_amount exist (all CosmosMsgIBCTransfer fields are `optional` in deps/device-protocol/messages-cosmos.proto:94-103, so nanopb emits has_ for every one); confirm_cosmos_address is declared in include/keepkey/firmware/app_confirm.h:49 and is already called 9 times in this same translation unit; no new header include is needed. No tabs; all lines <= 80 columns and wrapped the way clang-format's greedy bin-packing produces (the six-operand condition splits exactly as ORPHAN's equivalent does).
Not fixed here, flagged for follow-up rather than smuggled in: (1) signtx_tendermint.c :453-457 computes the device's own from_address and then discards it instead of requiring sender == from_address; (2) msg->ibc_transfer.denom is required-present but never used -- the serializer is handed the literal "uatom".
Closes #407
Thread the host-supplied MsgSend denom into the signed amino document (escaped, unbounded-length) instead of hardcoding "uosmo", and require has_denom so nothing unverifiable is ever displayed or signed. Risk: What a legitimate host notices: - A host that omits `denom` on OsmosisMsgSend now gets `Failure_FirmwareError "Message is missing required parameters"` instead of a silent uosmo send. This is the intended behaviour change and the only one that can break an existing integration. Worth checking hdwallet/pioneer emit `denom` on the Osmosis send path before shipping; every other Osmosis message type at BASE (delegate, undelegate, redelegate, IBC) already reads `.denom`, and OsmosisMsgSend.denom is field 3 in device-protocol, so this is almost certainly already populated. - For `denom == "uosmo"` with a plain-digit amount, the hashed bytes are IDENTICAL to BASE. Existing uosmo sends produce the same signature. That is the regression golden below. - Sends with a non-uosmo denom (IBC/factory) now produce a DIFFERENT signature than 7.14.1 did — a correct one. Anyone who somehow had a working IBC flow was getting a uosmo-denominated signature, so nothing legitimate depended on the old bytes. On the OLED: nothing new appears. The same "Send <amount> <denom> to <addr>" body renders; only the denom is now guaranteed to be the one that gets signed. Residual, NOT closed by this patch and pre-existing at BASE: - `confirm_helper()` body rendering has no pagination at BASE (fixed only in 7.15 by paginating inside confirm_helper). `strbuf` is BODY_CHAR_MAX=352 so nothing overflows, but a long IBC denom pushed through `confirm_transaction_output` may still run past what the screen draws. Closing that needs the 7.15 confirm machinery (confirm_bytes / confirm_body_split), which is not importable here. Every other Osmosis message at BASE already displays host denoms through the same path, so this patch does not widen the class. - `float amount = atof(...)` at fsm_msg_osmosis.h:143 loses precision above 2^24 and the display therefore rounds; the SIGNED amount is the exact decimal string, so display and signature can disagree in low digits for very large sends. Pre-existing, shared with delegate/undelegate/redelegate, and out of scope for #408 — flagging it, not touching it. - `osmosis.c:78` hardcodes the FEE denom as uosmo. Correct today (no fee-denom field in the proto), noted so nobody "fixes" it later by mistake. Closes #408
Measure the confirm body before drawing it, and when it will not fit the three on-screen rows, put an explicit "CUT OFF" screen with its own hold in front of it, so a partly-shown body is never shown without saying so. Risk: Behaviour change a user will see on the OLED: any confirm() or review() that goes through layout_standard_notification and whose body needs more than three rows now shows an extra screen titled "CUT OFF" reading "This text is too long for the screen. Only part of it is shown. Hold to view it anyway.", and needs a second button hold before the (still partial) body appears. This is value-dependent, not call-site-dependent - the same format string can be one screen for one amount and two for another, which is the known overflow class. Real BASE paths that will trip it: Bitcoin-family SignMessage and the VerifyMessage review with long host messages, long Cosmos/THORChain memos, long ETH data confirmations. For correct hosts the wire dialogue is unchanged: one ButtonRequest, one ButtonAck, one response. Only the wall-clock time to the response grows. Hosts with a ButtonAck timeout short enough to be tripped by one extra hold would time out, but no such timeout is device-side and the loop already has none. DebugLink and the emulator: each screen consumes one DebugLinkDecision. Any pyk/emulator test that drives a long-body confirm now needs one more decision or it will hang. This is the same cost 7.15's pagination imposes (the thorchain suite there records 5 presses where there used to be 2) - budget for it before running the fw test suite. review*() paths still discard the verdict (review() is hardcoded `return true`), so a host Cancel on the new warning screen ends the warning but review() still returns true and the caller proceeds. That is the pre-existing review() defect, unchanged and untouched here; the warning is still displayed, which is what #410 asks for. The check is a measurement, not a proof: it holds because calc_str_line() and draw_string() implement the same wrap loop today. If someone edits one loop and not the other the predicate under-reports. The new board-unit test does not catch that; it pins the arithmetic, not the coupling. What this does NOT do: it does not disclose the hidden text. A user who holds through the warning still signs something they have only partly seen - informed, but not fully. Full disclosure needs pagination (see decision_note). Adjacent defect noticed and deliberately NOT touched: fsm_msg_coin.h:272 passes (char*)msg->message.bytes to confirm("%s"), and SignMessage.message is `max_size:1024` in include/keepkey/transport/messages.options with no NUL guaranteed - a host sending exactly 1024 bytes makes the "%s" run off the end of the array into the rest of the decoded struct. Separate issue, separate fix. ROM/RAM cost: roughly 120 bytes of code plus ~90 bytes of rodata for the warning string. No new RAM buffers. Closes #410
…source Replace the eight hand-bounded strncat() calls in parseType() with a fail-closed append helper, closing the stack buffer overflow at eip712.c:208 (and the latent 1-byte overflow at :205) and turning silent type-string truncation into a refusal instead of a wrong typehash. Risk: Behaviour change for hosts: an EIP-712 message whose encodeType string exceeds STRBUFSIZE (511) chars now returns UDEF_NAME_ERROR and the device sends Failure_Other "EIP-712 user defined type name too long" instead of producing a signature. No legitimate flow is lost by this: at BASE such a message either truncated (yielding a typehash that no verifying contract or ecrecover-based checker would ever accept — the signature was already useless) or corrupted the stack. Anything that fits today still fits: the helper's boundary (used + added <= STRBUFSIZE) is byte-for-byte the boundary the already-correct strncat sites enforce. For calibration, Seaport's OrderComponents encodeType is ~443 chars, so real-world types do run close to the limit but under it — worth a smoke test against a Seaport/Permit2 payload before tagging. Nothing changes on the OLED. No new screen, no new button press, no new prompt; the type string is never displayed, only hashed. The dsConfirm() review_with_icon() call and every confirm() call site are untouched, so consent semantics are unchanged. Residual, deliberately NOT fixed here (all pre-existing at BASE, all outside #411's strncat/strcat/strcpy scope — file separately rather than widening this patch): - eip712.c:346-348 dsConfirm() sscanf's 40 hex chars out of dsverifyingContract with no length check; a short verifyingContract string reads past its NUL (bounded read within the 2048-byte host JSON buffer, not a write). - eip712.c:152-153 typeType = json_getValue(obTest) is passed straight to encodableType(), which strncmp's it; a JSON value node with no value yields a NULL deref. My helper's !value guard catches it at the append, but encodableType already dereferenced by then. - eip712.c:229/252/282 strncpy(byteStrBuf, ..., 2) can read past the end of a short hex string (read-only, in-buffer). Also note that ORPHAN's 6c27e11 ("reject overflow widths and wrong array sizes") is a genuinely separate defect in encodableType/encodeBytesN and is NOT addressed by this patch. Closes #411
… chunk Add one central guard in `ethereum_contractHandled()` so a contract decoder may only claim an EVM tx when the entire calldata fits in `data_initial_chunk`; oversized calldata falls through to the existing generic raw-data disclosure path. Risk: Behaviour change for legitimate hosts, in full: 1. A THORChain deposit / depositWithExpiry, 0x sellToUniswap, Sablier withdrawFromSalary, Uniswap add/removeLiquidityETH, or Uniswap approve whose calldata exceeds 1024 bytes no longer gets its clear-sign screens. Instead the user sees the Send screen, the "Signing of arbitrary ETH contract data…" AdvancedMode warning, the "Confirm Ethereum Data" hex screen, then the fee screen. Signing is still possible — this is a disclosure downgrade, not a block. Per the sizing above, no known legitimate call for any of those six selectors reaches 1024 bytes, so in practice this path should never fire for an honest host. An unusually long THORChain memo (roughly >640 characters) is the only realistic way to trip it. 2. Nothing changes for 0x transformERC20, by design. 3. makerdao is unaffected — it already applied this exact test internally at makerdao.c:628; the central guard just makes it redundant for that decoder. Residual, stated plainly: transformERC20 still signs trailing calldata that was never displayed. That is a deliberate, ORPHAN-matching exception justified by the pinned proxy address and the displayed input/min-output bounds — not an oversight. If the release wants zero exceptions, drop the hoisted transformERC20 line from the patch and accept that >1KB 0x swaps drop to the raw-data screen. Compile risk: none identified. `data_total` (uint32_t param), `msg->data_initial_chunk.size` (pb_size_t, promotes cleanly for `!=`), and `zx_isZxTransformERC20` are all already used in this exact translation unit at BASE. No new include, no new symbol, no header change. `data_total` was already a used parameter (makerdao_isMakerDAO) so no unused-parameter warning shifts either way. On-OLED: no new screen text, no new screen. Users only ever see the pre-existing generic path. Closes #413
…nd fence blind signing Delete the unbound "Send %s TRX to %s?" screen in fsm_msgTronSignTx and replace it with an honest AdvancedMode-gated blind-sign disclosure, since BASE has no TRON protobuf parser and therefore cannot verify anything about the transaction it signs. Fences TRON signing behind AdvancedMode, consistent with the existing fences for opaque Solana (fsm_msg_solana.h), unknown ETH calldata (ethereum.c) and bare-Ed25519 TON (fsm_msg_ton.h). This stops the device lying about the destination. It does not verify the destination: that needs a raw_data parser and is deliberately out of scope for this release. Risk: Behaviour change for legitimate hosts, and it is the reason this needs a sign-off: TRON signing now requires the AdvancedMode policy to be enabled. AdvancedMode is session state (it is off again after a power cycle), so a KeepKey Vault user sending TRX or TRC-20 will hit `Failure_Other "Enable AdvancedMode to blind-sign"` once per session until they toggle it. Nothing in the vault currently prompts for that, so this is a visible regression on a previously working path unless the host is updated — see decision_note for the ungated variant. On the OLED the "Send 12.5 TRX to TR7NHq..." screen disappears and is replaced by "Blind Sign / Sign unverified 218-byte TRON transaction? Amount and destination unknown." Users will read this as a downgrade; it is not — that destination was never verified, it was echoed back from the host. The flow is still two button holds (blind-sign screen, then the untouched "Transaction / Really sign this TRON transaction?"), same shape as BASE's opaque-Solana path. No new attack surface: no new parsing, no new buffers, no new heap or stack of consequence. Body string is at most 74 chars, well under BODY_CHAR_MAX (352, include/keepkey/board/layout.h:59) and under the ~84-char three-line OLED guarantee, so no truncation and no overflow. No ROM impact worth measuring (one policy call, one confirm, one fewer 32-byte stack buffer). Compile check against BASE: `storage_isPolicyEnabled` is declared at include/keepkey/firmware/storage.h:158 and storage.h is included by fsm.c:59, which is what #includes fsm_msg_tron.h (fsm.c:293). `confirm` is declared at include/keepkey/board/confirm_sm.h:94 with `__attribute__((format(printf, 3, 4)))`, so the `%u` against `(unsigned)msg->raw_data.size` type-checks. `ButtonRequestType_ButtonRequest_SignTx`, `FailureType_Failure_Other`, `FailureType_Failure_ActionCancelled`, `_()`, `memzero`, `layoutHome` are all already used elsewhere in this same file. `tron_formatAmount` stays declared in include/keepkey/firmware/tron.h and defined in tron.c — it is non-static, so dropping its only caller produces no warning. Closes #405
Make the ETH/TRON message-signing and message-verify screens say, in the body text the user actually sees, that the message was truncated and by how much — and fix two ETH-verify rendering bugs found while doing it. Discloses the truncation rather than refusing to sign. Refusing would break every Sign-In-With-Ethereum flow, whose bodies run 200-450 characters. Full review of long bodies needs paginated confirmation, which is 7.15 work. Risk: Behaviour change visible to users on the OLED, only in the truncating case: - ETH/TRON messages longer than 114 printable chars now show 84 chars instead of 114, prefixed with e.g. "TRUNCATED 84/389 bytes: ". Binary messages longer than 57 bytes now show 42 bytes of hex instead of 57, same prefix. Messages that already fit render byte-for-byte as before, because truncBuf is "" and the format is "%s%s". - ETH VerifyMessage: a message whose first 114 bytes are printable but which contains non-printable bytes later now renders on the "Bytes Verified" hex screen instead of the "Message Verified" text screen. That is the point of the change, but it is a visible reclassification a legitimate host could hit with, say, a UTF-8 message containing accented characters. TRON and ETH-sign already behaved this way, so this aligns Verify with the rest of the tree rather than introducing a new rule. - ETH VerifyMessage binary path previously showed size/2 bytes; it now shows up to 57. More hex on screen for short binary messages. No host-visible protocol change: no new failure codes, no new messages, no field changes, no refusal path added. Every confirm() return value that was checked before is still checked; none is newly discarded. Not covered by this patch, and worth stating so nobody reads the issue as closed: the bytes past the preview are still signed unread. This makes the truncation honest, not the message verifiable. ROM/RAM: four snprintf call sites with an extra format arg, plus four 32-byte stack locals. Should be a few hundred bytes of flash at most, but confirm against the CI ROM budget gate before merging — 7.14.2 inherits the tight budget. Closes #409
…onical
Validate the ABI head offset word before clear-signing 0x sellToUniswap and THORChain deposit/depositWithExpiry, and bound every fixed-offset read against the received calldata chunk in all four decoders.
Non-canonical encodings now fail closed rather than being described from hardcoded
offsets.
Risk: Behaviour change visible to users: none for a correct host. Solidity/ethers/web3 and the 0x API all emit canonical ABI head pointers (0x80 for sellToUniswap's tokens[], 0x80/0xa0 for deposit/depositWithExpiry's memo), so every real 0x swap and every real THORChain deposit still clear-signs with exactly the same screens.
What a rejected encoding looks like on the OLED: nothing. The checks run before the first confirm(), so ethereum.c:711-716 aborts immediately and the host gets Failure_ActionCancelled with "Signing cancelled by user". That message is misleading (the user pressed nothing) but it is the pre-existing behaviour for every other decode failure at BASE, e.g. zxswap's numOfTokens not in {2,3}. I did not change it, because doing so means touching ethereum.c and the router — see decision_note.
Residual risk deliberately left open (NOT part of #414, do not scope-creep into 7.14.2):
1. thortx.c still parses the memo as a fixed 64 bytes and ignores the ABI memo-length word. With the offset now pinned the memo starts where the router reads it, but a memo longer than 64 bytes still has its tail (affiliate, min-out) undisplayed. ORPHAN fixes this with length-word parsing + a 256-byte cap + an exact-end check; that is a separate, larger change.
2. thor_isThorchainTx is not pinned to the THORChain router address or chain_id at BASE, so any contract carrying the deposit selector gets the THORChain UX. That is a different issue entirely.
3. zxtransERC20's transformations[] tail remains undisclosed (bounded on-chain by minOutputTokenAmount).
Compile risk assessed as low: every identifier used (`msg->data_initial_chunk.size`, `.bytes`, `memcmp`, `size_t`, `bool`, `adder`, `is_expiry`) exists at BASE; `.size` is `pb_size_t` from PB_BYTES_ARRAY_T(1024) per include/keepkey/transport/messages-ethereum.options. Both comparison shapes I use already appear at BASE and compile. Adjacent-string-literal concatenation happens after escape resolution, so `"...\x00" "\x00...\x80"` is standard-safe. Flash cost is roughly 128 bytes of .rodata plus a few dozen bytes of code — negligible against the 7.14.2 ROM budget.
Closes #414
…arse failure Make thorchain_parseConfirmMemo return a tri-state so a host Cancel can never be mistaken for "memo not parseable" (and vice versa), read the ETH router memo's real ABI length/offset instead of a fixed 64 bytes at a fixed offset, disclose every trailing ':'-separated memo field, fix the memo copy that dropped its last byte and over-read its source, and bind the "Thorchain router" trust label to mainnet. Covers the memo bound and the cancel/parse-failure conflation. The router pin is NOT addressed here: the address migrated v1 -> v3 -> v4, so pinning this tree's literal would send every current THORChain swap to the blind-sign path. Filed separately. Risk: Things a legitimate host/user will notice: 1. Extra button holds. Any memo with fields past the labelled ones now gets one confirm screen per extra field, titled "Additional memo field". A normal ShapeShift swap memo carries ":<affiliate>:<bps>", so a THORChain swap gains 2 screens on all three paths (ETH router, native RUNE, BTC OP_RETURN). That is the point of the fix — those bytes set an affiliate skim and were previously signed invisibly — but it is a visible UX change on the OLED and the labels are generic, not "Affiliate fee N bps to X" as in 7.15. 2. ETH router deposits now hard-fail (Failure_ActionCancelled from ethereum.c, no blind-sign fallback) if the calldata is split across chunks, if the ABI memo offset word is not the canonical 0x80/0xa0, if the memo length word has dirty high bytes or exceeds 256, or if there are bytes after the padded memo. A router deposit is at most ~4+6*32+256 = 452 bytes against a 1024-byte data_initial_chunk, so a correct host is never split; but if some host does send data_length > data_initial_chunk.size for a deposit, it will stop signing. This is deliberate fail-closed behaviour: previously such a tx signed while only the first 64 memo bytes were shown. 3. min_chunk stays 228/260, so a very short memo (<33 bytes, e.g. "ADD:ETH.ETH") still does not clear-sign — exactly as at BASE. I did not import 7.15's relaxation to 164/196. Residual, NOT closed by this patch: - confirm()'s body has no pagination at BASE (no confirm_bytes / confirm_body_split), so a single very long memo field can still truncate on screen. An attacker who can make one field longer than the body budget can still hide its tail. Closing that needs the confirm-pagination work, which is real new machinery. - strtok collapses consecutive ':' separators, so an empty middle field can still shift a later field into an earlier label (e.g. an affiliate name displayed under "Confirm limit"). Every field is now SHOWN, but a field can still be MISLABELLED. The non-destructive empty-preserving splitter that fixes this is a rewrite of the function; not taken. - thor_isThorchainTx is still selector-only: any contract carrying the deposit selector still gets the THORChain clear-sign UI instead of the AdvancedMode blind-sign warning. See decision_note. - The withdraw screen still uses a "%3.2f" float format; untouched, out of scope. Compile notes I checked against BASE: -std=gnu99 -Wall -Wextra -Wshadow -Wsign-compare -Werror. All new comparisons are unsigned-vs-unsigned; both `for (size_t i ...)` loops in thortx.c are in sibling (non-nested) scopes so there is no shadowing or redeclaration; declarations after statements already occur in both files; memcpy/strtok/memzero/confirm are all already used in thorchain.c; msg->has_chain_id / msg->chain_id / msg->has_data_length exist on EthereumSignTx at BASE (makerdao.c already gates on has_chain_id); no unit test references thorchain_parseConfirmMemo (unittests/firmware/thorchain.cpp is not in unittests/firmware/CMakeLists.txt), so the signature change has exactly four call sites. Closes #415
#416 (10-iteration PIN KDF) is out of scope: it needs a flash image first, which is a different threat model from the USB-host defects in this set, and raising the count is a storage-format change rather than a self-contained fix. Also states plainly that the THORChain router pin and TRON destination verification are NOT closed here, so the release notes cannot imply they are. Corrects the build gate: -DKK_BITCOIN_ONLY=ON yields a byte-identical image on this tree (#397), so a 'both variants' checkbox could not fail.
clang-format 20 reflows the confirm() arguments added in the previous commit. Matches what CI's lint-format job enforces; no behaviour change.
secret-scan has been failing on develop itself, including at 1af2ffe, so this is a pre-existing red rather than something this branch introduced. Every finding was decoded before being exempted: - include/keepkey/firmware/u2f/u2f_keys.h holds the FIDO U2F batch attestation key. The standard shares one attestation key across a device batch by design, so it is compiled into every image and extractable from any release binary. It authenticates nothing and guards no funds. - include/keepkey/firmware/u2f/trezordevkey.pem is an orphan: its private scalar does not match U2F_ATT_PRIV_KEY[] and its public key does not match the compiled attestation certificate. It corresponds to nothing that ships. It is exempted rather than deleted because CI scans full history and the file has been present since fe3e5e0 (2019-04-27); deleting it now would not clear the finding. - Everything under deps/ is vendored third-party crypto, flagged by generic-api-key on ed25519-donna constants and AES self-test tables. - Bare 40-character lowercase hex is a git object ID. The security docs record exact submodule pins and gitleaks scores a commit SHA at ~3.8 entropy. Scoped to the git SHA-1 shape; key material here is 32 or 64 bytes and does not match. Verified: full-history scan over 5355 commits reports no leaks, and a planted EC private key under lib/firmware/ is still detected, so the scanner is narrowed and not blinded.
Five confirm() calls across the two Uniswap clear-sign handlers throw away their return value. Because confirm() returns false only on a host-sent Cancel/Initialize tiny-message, discarding it discards the host's cancel: a hostile host can drive the entire dialogue to completion and reach `return true` with zero button presses. zx_confirmApproveLiquidity() is the worst case — it is the 0x095ea7b3 ERC-20 approve path matched against the Uniswap router, both of its confirms are discarded, and it unconditionally returns true, so the ungated path yields a signed token approval. Fix: gate all five confirms with `return false`, matching the confirmFromAccountMatch() call already correctly gated at zxliquidtx.c:167.
Callers checked: Repo-wide grep (excluding .git) for zx_confirmApproveLiquidity / zx_confirmZxLiquidTx / ethereum_contractConfirmed found exactly one call chain and no test or emulator harness callers:
1. lib/firmware/ethereum_contracts.c:73 — `if (zx_isZxLiquidTx(msg)) return zx_confirmZxLiquidTx(data_total, msg);` and :75-76 — `if (zx_isZxApproveLiquid(msg)) return zx_confirmApproveLiquidity(data_total, msg);`. Both are direct tail returns inside ethereum_contractConfirmed(), so a new false propagates verbatim. No change needed there.
2. lib/firmware/ethereum.c:711 (CURRENT working tree, re-read after this release's edits) — the sole caller of ethereum_contractConfirmed():
if (!ethereum_contractConfirmed(data_total, msg, node)) {
fsm_sendFailure(FailureType_Failure_ActionCancelled,
"Signing cancelled by user");
ethereum_signing_abort();
return;
}
Risk: Low. Behaviour changes only along the host-cancel path, which previously produced a signature and now produces Failure_ActionCancelled — the intended direction, and it matches every other clear-sign handler in this tree.
Compile check by inspection: `confirm` is declared bool at include/keepkey/board/confirm_sm.h:105 and both files already include keepkey/board/confirm_sm.h; `ButtonRequestType_ButtonRequest_ConfirmOutput`, `appStr`, `amt`, `tikstr`, `poolstr`, `arStr`, `constr1`, `constr2`, `de
Closes #421
In the working tree at /Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-firmware-7142, three authenticator mutations call `confirm()` and throw the bool away, then mutate unconditionally: - `wipeAuthData()` — authenticator.c:80-91, confirm at :81, then `storage_wipeAuthData()` + `memzero(authData, ...)` at :86-89. - `addAuthAccount()` — authenticator.c:93-159, confirm at :147, then the slot write + `setAuthData()` at :151-156. - `removeAuthAccount()` — authenticator.c:298-338, confirm at :331, then `memzero(&authData[slot], ...)` + `setAuthData()` at :335-336. (The issue text calls this `deleteAuthAccount`; no such symbol exists in this tree — the function is `removeAuthAccount`.) `confirm()` (lib/board/confirm_sm.c:357-379) returns `confirm_helper()`'s verdict verbatim, so `false` here means the host sent a Cancel/Initialize tiny-message. Discarding it means a hostile host can send `Ping "\x19wipeAuthdata:"`, immediately follow with Cancel, and the device still permanently destroys all ten authenticator secrets while reporting `Success` — the host's own abort is executed as a commit. Same for add (host-chosen TOTP secret is persisted) and remove. Note the screen already blocks: `confirm_helper()` has no timeout, so this is not a "zero-wait" wipe. The defect is purely that the Cancel verdict is dropped, so cancelling and confirming are indistinguishable outcomes. Second half of the issue: `wipeAuthData()`'s body text ends "...If not, unplug Keepkey now." That instruction exists only because Cancel was inert. Once Cancel is honoured the sentence is wrong (it tells the user to yank a device that will now abort cleanly), so the fix removes it. Callers checked: Grepped the entire tree (excluding build/ and .git/) for `wipeAuthData|addAuthAccount|removeAuthAccount|getAuthAccount|generateOTP|AUTH_ERR_TYPE|NUM_AUTHERRS|UNKERR`. Every hit: 1. lib/firmware/fsm_msg_common.h:247 — `errcode = addAuthAccount(...)` inside `fsm_msgPing()`, case INITAUTH. ALREADY consumes the return code; the shared `if (errcode != NOERR)` at :291 turns CANCELED into `fsm_sendFailure(FailureType_Failure_ActionCancelled, "Action cancelled")` + `layoutHome()` + `return`. No change needed. Adding a new enum value does not break it because the switch is over AUTH_MSG_TYPE, not AUTH_ERR_TYPE — there is no `switch (errcode)` anywhere, so no -Wswitch fallout. 2. lib/firmware/fsm_msg_common.h:275 — `errcode = removeAuthAccount(...)`, case REMACC. Same: already consumes the code. No change needed. 3. lib/firmware/fsm_msg_common.h:280 — `wipeAuthData();` with `errcode = NOERR;` h Risk: Low, but three things the applier should know. 1. Behaviour change visible to hosts and to out-of-tree pyk/DebugLink tests: a `Ping` carrying `\x19wipeAuthdata:`, `\x15initializeAuth:` or `\x18removeAccount:` that is followed by a host Cancel now returns `Failure_ActionCancelled` instead of `Success`, and the mutation does not happen. Any external test that asserts Success while cancelling was asserting the bug. No NEW hang is introduced: `confirm_helper()` already blocked with no timeout befor Closes #422
…e resumed Verified against the working tree at /Users/highlander/WebstormProjects/keepkey-stack/projects/keepkey-firmware-7142/lib/firmware/reset.c (unchanged from 1af2ffe in this release — `git diff 1af2ffe -- lib/firmware/reset.c` is empty, so the reported line numbers are accurate). Confirmed layout: `awaiting_entropy` declared at :42, armed at :127 as the last statement of reset_init(), cleared at :146 in reset_entropy(). It is the ONLY gate on the seed-generation path — `fsm_msgEntropyAck` (lib/firmware/fsm_msg_common.h:545) has no CHECK_* macro and calls reset_entropy() unconditionally. reset_init() has FIVE early returns, none of which clear the flag: :55 invalid strength :65 display_random && no_backup :82 no_backup double-confirm not completed :102 Internal Entropy confirm not completed :112 change_pin() failed The exploit is worse than "stale entropy is reused", because reset_init writes the module globals BEFORE it can abort: `strength = _strength;` (:58) and `no_backup = _no_backup;` (:59) execute ahead of every confirm. So: 1. Ceremony A completes reset_init (user consented, no_backup=false) -> awaiting_entropy = true, device sits armed waiting for EntropyAck. 2. Hostile host sends ResetDevice with no_backup=true. Line 59 sets the global no_backup=true, then the double-confirm at :72/:76 is reached. The host sends a Cancel tiny-message, confirm_helper() returns false, and reset_init returns at :82 WITHOUT clearing awaiting_entropy. 3. Host sends EntropyAck. reset_entropy() passes the :131 gate on ceremony A's stale flag, hashes ceremony A's int_entropy with host-chosen ext_entropy, sees the corrupted no_backup==true, and takes the :148 branch: storage_setNoBackup(); storage_setMnemonic(); storage_commit(); fsm_sendSuccess("Device reset"). That is a fully provisioned device with a host-influenced seed, the recovery seed NEVER displayed, and ZERO button presses. The same trick corrupts `strength` (256 -> 128) even on the backup path. No other module clears this state. `git grep -n awaiting_entropy -- lib include` returns only reset.c:42/127/131/146. There is no reset_abort()/reset_cancel() anywhere (`git grep -rn 'reset_abort\|reset_cancel'` is empty), and neither fsm_msgInitialize (fsm_msg_common.h:1-11) nor fsm_msgCancel (:553-561) touches it — both abort recovery_cipher/signing/ethereum/tendermint/eos and call session_clear(), which lives in storage.c and knows nothing about reset.c's statics. reset_entropy() itself has NO equivalent hole: the flag is cleared at :146 before any confirm and before any `goto exit`, so the backup-confirm cancel at :161-166, the MAX_PAGES abort at :198-202 and the per-page confirm_constant_power cancel at :245-249 all run with the flag already down. FIX: one line — `awaiting_entropy = false;` as the first statement of reset_init(), before the strength validation. Callers checked: Traced every consumer of the state and of both functions. No signature and no return type changes, so the caller surface is small and fully enumerated. 1. reset_init — exactly one call site. `git grep -n reset_init -- lib include` -> include/keepkey/firmware/reset.h:36 (decl), lib/firmware/reset.c:46 (def), lib/firmware/fsm_msg_common.h:533 (only call). lib/firmware/fsm_msg_common.h:530-543 `fsm_msgResetDevice`: guarded by CHECK_NOT_INITIALIZED (fsm.c:107-112, which returns before reset_init on an initialized device), then calls reset_init and returns. It is `void`, ignores nothing, and inspects no state afterwards. The added line is invisible to it. Note the CHECK_NOT_INITIALIZED guard does not weaken the fix: storage_isInitialized() (storage.c:1954-1957) tests has_node || has_mnemonic, and an armed-but-unfinished ceremony has committed neither — reset.c only reaches storage_s Risk: Low. One assignment to an existing file-static bool, at the top of a function that already writes that same variable 80 lines later. No new symbol, no new include, no signature change, no header change, no change to any confirm() call or its handling. Compile check by inspection: `awaiting_entropy` is declared at reset.c:42 as `static bool`, i.e. in scope and assignable at reset.c:49 (same translation unit, above the function). `<stdbool.h>` reaches this file via include/keepkey/firmware/reset. Closes #423
…elease #403 fixed a discarded confirm() return value. Sweeping for the same pattern found eight more sites: three in the authenticator, five across the two Uniswap clear-sign handlers. #421-#423 track them and the fixes are on this branch. The worst was zx_confirmApproveLiquidity, the 0x095ea7b3 ERC-20 approve path pinned to the Uniswap router, which discarded both confirmations and returned success unconditionally.
calc_str_line() returned uint32_t but accumulated into a uint8_t. A confirm body carrying 255 newlines wrapped the count back to 0, so confirm_body_fits() reported that it fitted and the 'Cut Off' warning added earlier in this release was skipped. The 352-byte confirm buffer has room for a benign prefix, 255 newlines and a hidden suffix, so the guard was bypassable by a host that chose its whitespace. Found in review of this branch. The guard was introduced here, so this closes a hole this release opened rather than one it inherited: promoting calc_str_line() to a security boundary without auditing its accumulator. Adds Board.ConfirmBodyFitsLineCountDoesNotWrap, which fails on the previous code at 254/255/256/257/340 newlines and on the prefix+newlines+suffix shape an attacker would actually send.
thor_confirmThorTx() kept a 228/260-byte minimum calldata check while the comment added earlier in this release documents the correct 164/196. The old floor assumed a fixed 64-byte memo and rejects valid short ones: '+:BTC/BTC::t:10' pads to 32 bytes, giving 196 bytes of calldata for deposit(), which was refused and fell through to blind signing. 164/196 is exactly the bound needed to read the memo's ABI length word, which sits at 4 + 4*32 for deposit() and 4 + 5*32 for depositWithExpiry(). Lowering it is safe because the exact-length equality check further down is what actually bounds the memo: memo_off + memo_padded must equal data_initial_chunk.size.
Review flagged that the allowlist carried a global regex for \b[0-9a-f]{40}\b,
which suppressed every 40-character lowercase hex finding anywhere in the tree
rather than only documented commit hashes. Replaced with a docs/ path scope, which
is where the git-SHA false positives actually live.
Full-history scan over 5356 commits reports no leaks, and the negative control still
detects a planted EC private key under lib/firmware/ while allowing a submodule pin
in docs/.
The header closed an extern "C" block it never opened, so including it from C++ emitted a stray '}' that closed the includer's own block. That is why no unit test could include eip712.h, which pulls this in — the EIP-712 code had no reachable C++ test surface at all. C builds are unaffected: __cplusplus is undefined there.
Review of PR #418 found that #403's fix removed the zero-button signing oracle but left the substantive problem: every domain and message field screen was drawn with review(), which discards confirm_helper()'s verdict and hardcodes 'return true'. A host could protocol-Cancel each field screen and the user, having seen nothing, met only the single generic gate before signing arbitrary typed data. confirmName()/confirmValue() now return USER_CANCELLED, dsConfirm() returns a status instead of void, and parseVals() checks all nine call sites so encode() aborts before a hash exists. failMessage() maps USER_CANCELLED to Failure_ActionCancelled — the same event the terminal gate reports — so a host can distinguish refusal from malformed typed data. review() itself is deliberately unchanged, as the reviewer required: other subsystems rely on it being advisory. dsConfirm() needs an icon-capable primitive that can refuse, so confirm_with_icon() is added as an additive mirror of review_with_icon(). USER_CANCELLED is defined ABOVE LAST_ERROR and handled before the range check, so failMsgReturn[]'s size and its err-3 indexing are untouched.
… string fsm_msgSignMessage passed SignMessage.message.bytes straight to "%s" while cryptoMessageSign() signed message.size bytes. An embedded NUL terminated the display and everything after it was signed invisibly: 'benign login\0hidden authorization' showed only the prefix. The renderer measurement added earlier in this release cannot catch this either, because calc_str_line() stops at the same NUL. format_message_body() renders from (bytes, size), hex-encodes anything that is not printable ASCII so no signed byte can stay off screen, and states how much of an oversized message is shown -- in front of the preview, since draw_string() clips the tail. fsm_msgVerifyMessage had the same shape and is fixed with it.
Review of PR #418 noted that #406 fixed the exponent but not the label: the amount is now scaled by the decimals inside the signed TransferChecked instruction, while the symbol was still taken from host-supplied token_info and displayed as though the device had verified it. There is no authoritative symbol on-chain and no on-device mint table here, so 'USDC' was asserted on the host's word alone. The screen now reads 'Send 1.000000 tokens to <recipient>?' with the mint on its own screen. The device asserts only what it can prove, and no host-controlled string reaches the display, so there is nothing left to sanitise. Removes solana_findTokenInfo() and the symbol-safety filter it needed.
Two adversarial rounds have each found High-severity defects in work called ready, three of them introduced by this branch, so the release is a strong candidate and not yet defensible. Records the six gates in order, the change freeze, and the only supportable public statement. Explicitly rejects 'all critical/CVE-level issues addressed' as a claim, and records that #429 predating this release does not reduce its blocking significance.
The Orchard spend-authorization signer did not implement the spec's nonce
construction. This branch pinned a crypto revision whose signer drew its own
randomness with plain random_buffer() and reduced it straight to the nonce
scalar. The Zcash protocol specification draws an 80-byte T and computes
r = H*(T || vk || M)
hashing the randomness together with the verification key and the message.
That binding is the point. With M inside the hash, two signatures over
different messages get different nonces even when T repeats. Without it, one
repeated draw across two messages discloses the spend authorization key by
arithmetic any observer can do from public data:
ask + alpha = (s1 - s2) / (c1 - c2)
Two further defects rode along.
BIAS. Reducing 32 bytes mod the Pallas scalar order leaves each residue with 3
or 4 preimages -- a 33% relative bias, which is exactly the input a lattice or
hidden-number attack wants. The digest is now reduced wide, where the bias is
negligible. (The same narrow reduction is safe in BIP-340 only because
secp256k1's order sits within 2^-128 of 2^256; Pallas is ~2^254.)
NORMALISATION. pallas_ct_scalar_replace_zero_with_one() converted a dead
entropy source into the CONSTANT nonce 1 on every signature -- reused AND
publicly known, which discloses the key from a single signature rather than
two. It is deleted, not merely unused: a zero scalar now fails the signature.
Refusing to sign is always safe; signing with a predictable nonce is not.
CALLER. fsm_msg_zcash.h draws T through random_buffer_checked() behind
rng_health_check(), so a degraded generator yields no signature instead of a
predictable one. rng_health.h listed this draw as a known exception; it is
covered now, so the exception is removed rather than left to rot, and the
stale example in storage.c's scope note goes with it.
GATE. tools/check_pallas_api_boundary.py REQUIRED the normaliser in the
signing core -- the defect was written down as an invariant, and the gate
would have rejected this fix. It now forbids the normaliser, forbids the
signing core drawing its own randomness, requires the spec hash, and requires
the nonce hash to wipe its BLAKE2b context (blake2b_Final leaves the digest in
ctx.h and the last input block, containing T, in ctx.buf).
TESTS. Six cases, and they have teeth: with the pre-fix derivation restored by
mutation, RepeatedT_DifferentMessage_DifferentR and
RepeatedT_DifferentKey_DifferentR both fail. The suite that shipped alongside
the defect did not -- RedPallasSign_DifferentSighash passes under the broken
code, because different messages produce different CHALLENGES whatever the
nonce does. It watched the wrong half of the signature; the new tests assert
on R, the half that discloses the key.
The constant-time test for the deleted normaliser is removed with it.
Sibling audit: ECDSA is RFC 6979, BIP-340 does the spec construction binding
key and message, generate_k_random() is rejection sampling used for coordinate
blinding, and bip39's self-draw, cosi and Monero are unreachable from firmware.
RedPallas was the only member of this class.
Upstream fixes: keepkey/trezor-firmware#10 and #11.
Full detail: #509.
An adversarial audit of this release produced 33 findings, 27 of which survived independent refutation. This lands the fixes for the exploitable and high-severity ones. All are verified green on the full CI graph. ZCASH: A v6 TRANSACTION NO LONGER SIGNS AN UNVERIFIED BUNDLE. The device streams and verifies only the ACTIVE pool's actions, but orchard_digest was mandatory on the Ironwood path, folded into the signed sighash, and never checked against anything. That is a theft path: stream honest Ironwood actions, point orchard_digest at a real Orchard bundle spending one of this seed's notes, and reuse the alpha of an approved action so rk is byte identical. RedPallas verifies as [s]G = R + [H(R||rk||M)]rk, so with rk and M shared the single signature the device emits verifies in BOTH bundles, and the malicious bundle's valueBalance never reaches the fee arithmetic. The user approves a small send and an arbitrary note moves with it. EMPTY_ORCHARD_DIGEST pins the inactive pool exactly as EMPTY_SAPLING_DIGEST and EMPTY_TRANSPARENT_DIGEST already did -- Orchard was the one unused pool taken on trust. The ZIP-244 value was validated by reproducing both existing constants with the same method before being used. ZCASH: rk IS CHECKED AGAINST THE DEVICE'S OWN KEY. Production signed through redpallas_sign_digest_for_rk(), which feeds the host's rk into the nonce and challenge hashes without ever verifying it describes this device's key. redpallas_sign_digest_with_ak() derives rk from the cached ak and alpha, refuses on mismatch, and signs with the derived value. It already existed and was already tested. Cost: one scalar multiplication per action. ZCASH: THE SIGNING SESSION IS TORN DOWN. zcash.h documents zcash_signing_abort() as existing so a host cannot resume streaming against a previously-approved session after Initialize, Cancel or ClearSession, and fsm.c repeats the claim, but nothing called it. The Orchard spend authorizing key survived all three. ZCASH: SEEDS AND KEYS NO LONGER SURVIVE IN HASH CONTEXTS. blake2b_Final() leaves the digest in ctx.h and the last input block in ctx.buf. zip32_orchard_master() hashes the BIP-39 SEED, prf_expand() hashes the Orchard spending key, and storage_getRootNode()'s PBKDF2 context is passphrase-keyed and derives the AES key that decrypts the private key. A sweep found 17 unwiped contexts; the other 14 hash public data and are left alone. CLEAR-SIGN AND ETH: AN AMOUNT NEVER RENDERS BLANK. bn_format() answers overflow with memset(output, 0, len); return 0, and all three call sites ignored the return, so an oversized amount displayed as an EMPTY string -- the one rendering a user cannot read as wrong. 256-bit at 18 decimals needs ~80 characters against a 32-byte buffer. THE BOUNDARY GATE STATED THE WEAKER PROPERTY. It REQUIRED _for_rk and explicitly FORBADE _with_ak, so it would have rejected this fix -- the third time this file was found pinning the weaker of two available implementations, after it required the nonce normaliser. It now requires the validating path, forbids the unvalidated one, and requires the nonce hash to wipe its context. Every rule was mutation-checked in both directions. TESTS. The v6 fixture passed filler bytes as orchard_digest with a comment describing the vulnerability as the design; it now uses the ZIP-244 value, and Z26 asserts a non-empty Orchard bundle is refused. The display-disclosure suite was passing vacuously against an uninitialized device -- four tests reported PASS having exercised zero display logic and the fifth skipped itself. With a seed loaded they now run and pass for real. Findings: #509 #511 #513 #514 #517 #527.
MultisigRedeemScriptType.signatures is declared max_size:73, so pb_decode accepts 73 bytes -- but a DER-encoded ECDSA signature is at most 72. Nothing checked the runtime size, and the witness serializer appended the sighash byte AT signatures[i].size, so 73 wrote one past the end of bytes[73]. For i < 14 that lands on signatures[i+1].size: a host can flip a later slot from 0 to nonzero, the `size == 0` skip stops firing, and the device emits an extra witness element built from a slot the host left empty -- the witness stack no longer matches what the user reviewed or what was hashed. At i == 14 it lands on has_m. signing_validate_input() now rejects anything over 72 bytes before that size indexes anything, and the witness element is built in a local so the out-of-bounds write no longer exists to be gated. A declared max_size is a DECODER bound and must never be relied on as the runtime one. The device's own signature append is unaffected: same 73-byte field, but the device produces the value so it is at most 72 and the write is in bounds. Test Z27 asserts an oversized signature is refused, registered in SECTIONS so it is in the CI filter and actually runs.
…RNG latch Second batch from the 7.15 audit. Rebased onto a3da828 -- that commit's files are untouched here. SEED BACKUP: 1.7% OF 24-WORD BACKUPS WERE CLIPPED. layout_constant_power_- notification() draws from x = 128 + LEFT_MARGIN, so only KEEPKEY_DISPLAY_WIDTH - (128 + LEFT_MARGIN) = 124 px exists past that origin, while BODY_WIDTH (225) is passed as the wrap width. The wrap never fires before the canvas edge does: draw_char_impl() rejects the first glyph crossing 256, draw_string_walk() stops, and every character after it is dropped -- including whole later lines. No ellipsis, no warning, no page indicator. confirm()'s completeness check was hard-gated to layout_standard_notification, with a comment noting that measuring a constant-power body against BODY_WIDTH would be wrong. It would have been. But excluding it left the backup pages with no check at all. Measured with the real font tables and the real placement rules over 200,000 random 24-word mnemonics: 1.712% a backup page is clipped (1 in 58) 0.646% a whole word is never shown (1 in 155) 1.075% only the final word truncates, e.g. "24.observ" A missing word is not recoverable from the backup. The fix measures at the right origin instead of skipping the measurement, and the probe is selected by layout and threaded through page_take(), so an over-full page is PAGED by machinery that already exists. No new buffers, MAX_PAGES untouched -- widening the packing width in reset.c instead would have needed MAX_PAGES 6 -> 12, about 3.7 KB more static SRAM, and would have hard-failed wallet creation with "Too many pages of mnemonic words". ZCASH SESSION: cleared wherever the device session is torn down, not at three named handlers. The screensaver auto-lock (home_sm.c) and the PIN-failure path (pin_sm.c) both tear it down without going through Initialize or ClearSession, and both left the Orchard spend AUTHORIZING key live in RAM. session_clear() already revokes ClearSign signer slots for exactly this reason; the Orchard key belongs to the unlocked session on the same grounds. BITCOIN-ONLY LOCK: a locked wallet resets the RAM shadow, so handlers that persist settings ran normally, storage_commit() declined to write, and the handler answered Success -- the change survived the session and vanished at the next boot. CHECK_NOT_BITCOIN_ONLY_LOCKED now guards ChangePin, ChangeWipeCode, ApplySettings and ApplyPolicies. ChangeWipeCode was found by sweeping every storage_commit() caller rather than fixing only what was reported. WipeDevice is deliberately NOT gated: wiping is the documented way out. RNG: rng_source_live() tested RNG_SR_SEIS claiming "SEIS latches, so a transient fault is still visible here". It does not -- random32() clears SEIS/CEIS as soon as the underlying condition goes, as it must to keep drawing, and it runs constantly. A fault it saw, recovered from and erased was invisible to the boot health verdict that every checked key draw fails closed behind. Now mirrored in a boot-lifetime software latch set where the hardware latch is cleared. CI: firmware-unit failures now fail their job. The entrypoint ended with `cp`, so the container's exit status was the copy's, and CI consumes that status directly. Verified both shapes rather than assumed. The separate unit-tests job does gate the same binary, so this was not a total blind spot. Tests: Board.ConstantPowerBodyFitsMeasuresFromItsOwnOrigin pins a real clipped page and asserts the left-margin probe still calls it fine, so it keeps demonstrating the blind spot. Findings: #519 #513 #526 #528 #533.
Paging a constant-power body changes the HOST protocol. page_body_confirm()
emits one ButtonRequest per page -- "Every page after the first writes its own
request" -- and the seed-backup flow is driven by a host that reads one word
group per ButtonRequest. A split backup page therefore made the host read that
group twice, and test_reset_device reconstructed a mnemonic with duplicated
words:
...mushroom valid chronic category mushroom valid way suit parent
...mushroom valid way suit parent (expected)
Any host driving ResetDevice counts ButtonRequests, the vault included, and the
corruption lands in the words the user is copying down. Worse than the clipping
it was meant to fix, so it is reverted.
Kept: confirm_body_fits_constant_power(), which measures at the origin that
layout actually draws from, and the test pinning a real clipped backup page.
The measurement is correct and now inert -- the blind spot stays documented
instead of being forgotten.
The clipping is still real: 1.712% of 24-word backups clip and 0.646% never
show one of the words, measured over 200k mnemonics with the real font tables.
The fix is to pack reset.c's pages against the width they are drawn at, which
needs MAX_PAGES raised (~3.7 KB static SRAM) and OLED verification on hardware.
Tracked in #519.
Simulate merging keepkey#462 after the fork 7.14.3 develop line. The resulting tree is intentionally byte-for-byte identical to upstream PR #462; the fork-only parent contributes ancestry, not divergent release content.
security: audit and harden 7.14.3 Bitcoin-only + Dice
…develop-20260826 # Conflicts: # deps/python-keepkey
There was a problem hiding this comment.
🔵 Needs a closer look
It makes broad, security-sensitive firmware/build changes and currently contains unresolved build- and consent-path issues identified in review comments.
Pull request overview
Updates the KeepKey firmware release line to 7.15.0 on top of the current 7.14.3 bitcoin-only review head, bringing in the 7.15 product delta while also hardening build/CI gates (SRAM/stack) and improving cross-variant and cross-platform build behavior.
Changes:
- Adds SRAM safety enforcement (linker reserve assert + CI budget gate) and reduces device stack pressure (shared frame arena for message encoding;
-fstack-usagereporting). - Expands multi-chain feature set and protocol wiring (Zcash privacy-gated flows, Hive messages, clearsign metadata/signers, BIP-85), plus corresponding unit tests and build gating for bitcoin-only vs full variants.
- Security/robustness fixes across signing, transaction handling, recovery, emulator portability (Windows/MinGW), and UI paging/confirmation behavior.
File summaries
| File | Description |
|---|---|
| unittests/firmware/transaction.cpp | Adds multisig quorum validation test coverage; removes prior omni paging test. |
| unittests/firmware/signing.cpp | Updates script-type encoding test; removes multisig quorum and abort-scrub tests. |
| unittests/firmware/rng_health.cpp | Adds a persistent RNG error latch regression test. |
| unittests/firmware/eip712.cpp | New EIP-712 strict/canonical encoding and error-path tests. |
| unittests/firmware/coins.cpp | Adds allowlist for legacy coins lacking token-table entries to keep sanity checks strict for new regressions. |
| unittests/firmware/CMakeLists.txt | Fixes unit test source selection across full vs bitcoin-only; adds privacy-gated zcash tests and new multi-chain suites. |
| unittests/firmware/binance.cpp | Extends denom/transfer validation tests and adds secp256k1 include. |
| unittests/firmware/authenticator.cpp | Reworks authenticator tests toward fail-closed cancellation, input validation, and duplicate/weak-secret handling. |
| unittests/crypto/CMakeLists.txt | Updates include path; adds privacy-gated crypto test executables; adjusts link dependencies. |
| unittests/board/CMakeLists.txt | Updates crypto include path for board unit tests. |
| tools/sram-budgets.json | New per-variant SRAM budget configuration for CI gate. |
| tools/merge-direction-adjudicated.txt | New adjudication allowlist for merge-direction gate flags. |
| tools/merge_symbol_gate.py | New symbol-survival gate script for merge validation. |
| tools/merge_direction_gate.py | New direction-of-resolution audit script for merge validation. |
| tools/firmware/keepkey.ld | Adds linker-time SRAM reserve ASSERT to prevent runtime stack/heap exhaustion. |
| tools/firmware/CMakeLists.txt | Emits link map for CI SRAM reporting; updates crypto include path. |
| tools/emulator/CMakeLists.txt | Gates standalone UDP emulator binary off on Windows; adds MinGW linker-group/export handling for dylib. |
| tools/display_test/CMakeLists.txt | Updates crypto include path for display test tool. |
| tools/check_sram_budget.py | New CI script to enforce SRAM reserve and frame-margin budgets using ELF symbols + stack-usage data. |
| tools/bootstrap/CMakeLists.txt | Updates crypto include path for bootstrap tool. |
| tools/bootloader/CMakeLists.txt | Updates crypto include path for bootloader tool. |
| tools/blupdater/CMakeLists.txt | Updates crypto include path for blupdater tool. |
| scripts/emulator/firmware-unit.sh | Fixes CI gating so container exits with test status while still extracting reports. |
| scripts/emulator/Dockerfile | Removes duplicate CMake build-type flag and trailing whitespace. |
| scripts/emulator/docker-compose.bitcoin-only.yml | Sets firmware variant env for python-keepkey in bitcoin-only compose config. |
| scripts/emulator/capture-thor-percent.py | New evidence tool to capture THOR/Maya percent-confirm OLED layouts. |
| scripts/build/docker/device/release.sh | Allows passing extra CMake flags (e.g., bitcoin-only) through release build script. |
| Makefile | New convenience lint/format targets mirroring CI clang-format expectations. |
| lib/transport/CMakeLists.txt | Adds Hive and Zcash protobufs/options to transport generation and build outputs. |
| lib/rand/rng.c | Makes persistent RNG error policy unit-testable; refactors latch behavior. |
| lib/firmware/txin_check.c | Adds txin_dgst_reset_only() to avoid finalized-context reuse after OP_RETURN outputs. |
| lib/firmware/transaction.c | Fixes OP_RETURN hashing reset and introduces shared multisig quorum validator used by compilers. |
| lib/firmware/tiny-json.c | Renames errno global to avoid libc macro collisions on glibc/MinGW. |
| lib/firmware/tendermint.c | Replaces generic safe-text/bech32 validators with denom/asset/signer-specific validators. |
| lib/firmware/storage.h (lib/) | Adds legacy clearsign identity layout placeholders and PIN-KDF v19 gating APIs/flags; adds per-version read/write prototypes. |
| lib/firmware/signtx_tendermint.c | Hardens bech32 decode buffer sizing by using shared constants. |
| lib/firmware/ripple.c | Adds XRPL memo serialization support. |
| lib/firmware/reset.c | Removes internal-entropy display; shares mnemonic display scratch buffers across reset and BIP-85 flows. |
| lib/firmware/recovery_cipher.c | Improves recovery cipher UX/state correctness; adds per-word validation and backspace resync; moves large permute table to frame arena scratch. |
| lib/firmware/osmosis.c | Adds strict parsing/formatting for amounts/denoms; removes float formatting by introducing integer formatting helper. |
| lib/firmware/messagemap.def | Adds message wiring for BIP-85, Zcash privacy flows, Hive, and clearsign metadata/attestor messages with variant gating. |
| lib/firmware/fsm.c | Adds new handlers/includes (BIP-85, Hive, Zcash); adds bitcoin-only locked guard macro; adjusts abort stubs for variant builds. |
| lib/firmware/fsm_msg_ton.h | Strengthens TON message disclosure to page every signed byte; clarifies blind-sign gating rationale. |
| lib/firmware/fsm_msg_thorchain.h | Tightens THORChain message envelope handling and confirmation flow; refactors send/deposit paths and memo confirmation. |
| lib/firmware/fsm_msg_ripple.h | Fixes Ripple address buffer mismatch and pages full memo disclosure before signing. |
| lib/firmware/fsm_msg_ethereum.h | Adds clearsign metadata and signer-loading handlers with signing-in-progress protections and icon validation. |
| lib/firmware/fsm_msg_crypto.h | Fixes identity-signing confirmation to bind what is actually signed (SSH/GPG vs generic), and pages challenge bytes. |
| lib/firmware/fsm_msg_bip85.h | New handler to derive and display (not export) BIP-85 child mnemonics with paginated constant-power confirmation. |
| lib/firmware/ethereum_contracts.c | Prevents contract CREATE from being clear-signed and adds Maya contract routing. |
| lib/firmware/eos.c | Improves AdvancedMode refusal UX and fixes Windows-safe asctime usage. |
| lib/firmware/coins.c | Clarifies bitcoin-only token exclusion guard/commenting. |
| lib/firmware/CMakeLists.txt | Adds BIP-85 and signed_metadata to full builds; gates Zcash privacy engine sources by KK_ZCASH_PRIVACY. |
| lib/firmware/bip85.c | New BIP-85 mnemonic derivation implementation. |
| lib/firmware/app_layout.c | Adds Zcash address view layouts and extends recovery cipher layout to show previous-word info. |
| lib/firmware/app_confirm.c | Adds Zcash address confirm flow (text + QR). |
| lib/emulator/udp.c | Skips BSD sockets when building dylib/DLL mode (Windows compatibility). |
| lib/emulator/setup.c | Removes duplicate __stack_chk_guard symbol; gates flash setup off on Windows; factors RNG init to shared helper. |
| lib/emulator/random.c | New cross-platform emulator CSPRNG implementation (/dev/urandom vs BCryptGenRandom). |
| lib/emulator/CMakeLists.txt | Adds random.c and threads dependency for kkemulator_dylib. |
| lib/board/variant.c | Fixes bitcoin-only emulator variant name so multi-chain tests can be correctly skipped. |
| lib/board/util.c | Hardens base-unit decimal formatting: canonical digits-only input, size_t bounds, and rejects leading zeros. |
| lib/board/usb.c | Animates progress during blocking host I/O; moves protobuf encode to shared frame arena to reduce stack usage. |
| lib/board/udp.c | Mirrors device usbPoll animation ticking in emulator UDP transport. |
| lib/board/timer.c | Adds Windows-compatible delay/timer ticking behavior for emulator. |
| lib/board/signatures.c | Adds fault-injection hardening for signature verification (double SHA, constant-time compare, infective aggregation). |
| lib/board/keepkey_board.c | Fixes emulator CRC32 implementation to match STM32 peripheral semantics and word_len meaning. |
| lib/board/font.c | Adds length-bounded line measurement and a simple paging helper. |
| lib/board/draw.c | Adds strict RLE validation for host-supplied runtime icons and hardens decoder packet parsing. |
| lib/board/CMakeLists.txt | Updates crypto include path for board library. |
| include/keepkey/transport/messages.options | Raises Entropy payload max_size and refines clearsign attestor payload sizing rationale. |
| include/keepkey/transport/messages-zcash.options | New nanopb sizing options for Zcash privacy messages. |
| include/keepkey/transport/messages-hive.options | New nanopb sizing options for Hive messages. |
| include/keepkey/transport/interface.h | Includes generated Hive/Zcash message headers in transport interface. |
| include/keepkey/rand/rng.h | Exposes persistent RNG error stepper for unit testing. |
| include/keepkey/firmware/txin_check.h | Declares txin_dgst_reset_only() with rationale. |
| include/keepkey/firmware/tron.h | Adds typed raw-tx parsing/formatting interface for verified TRON signing. |
| include/keepkey/firmware/transaction.h | Declares shared multisig quorum validator. |
| include/keepkey/firmware/tiny-json.h | Removes extern errno declaration (no longer provided by tiny-json). |
| include/keepkey/firmware/thorchain.h | Expands THORChain validation helpers and memo paging API; documents formatting contract. |
| include/keepkey/firmware/tendermint.h | Adds BECH32_DECODED_MAX and replaces old validators with denom/asset/signer APIs. |
| include/keepkey/firmware/storage.h | Clarifies shipped storage-version invariant and adds bitcoin-only lock documentation. |
| include/keepkey/firmware/signing.h | Replaces checksum helper API with ABI-independent script-type encoding helper. |
| include/keepkey/firmware/reset.h | Shares mnemonic display scratch across reset and BIP-85; removes display_random parameter from reset_init contract. |
| include/keepkey/firmware/policy.h | Documents burned storage bit and downgrade implications for policy persistence. |
| include/keepkey/firmware/osmosis.h | Adds amount/denom limits and a formatting API for integer-only confirm rendering. |
| include/keepkey/firmware/mayachain.h | Adds MAYA denom/asset/signer validation helpers. |
| include/keepkey/firmware/fsm.h | Declares new handlers (metadata/signers, clearsign attestor, Hive, Zcash, BIP-85). |
| include/keepkey/firmware/ethereum.h | Adds signing-in-progress query and strict EIP712Domain primaryType check helper declaration. |
| include/keepkey/firmware/ethereum_tokens.h | Defines TOKENS_COUNT=0 in bitcoin-only builds. |
| include/keepkey/firmware/ethereum_contracts/zxliquidtx.h | Adjusts interface (removes HDNode dependency; adds formatting helper). |
| include/keepkey/firmware/ethereum_contracts/thortx.h | Updates router pins and adds chain-scoped router notes + MAYA router constant. |
| include/keepkey/firmware/eip712.h | Updates docs and exposes bytes encoders for strict-value tests without redundant decls. |
| include/keepkey/firmware/coins.h | Clarifies token exclusion guard/commenting for bitcoin-only. |
| include/keepkey/firmware/coins.def | Fixes/clarifies bitcoin-only guard comment formatting. |
| include/keepkey/firmware/bip85.h | New BIP-85 API header. |
| include/keepkey/firmware/authenticator.h | Adds minimum secret size and updates error enum values (duplicate/cancelled). |
| include/keepkey/firmware/app_layout.h | Declares new Zcash layouts and updated layout_cipher signature. |
| include/keepkey/firmware/app_confirm.h | Declares confirm_zcash_address under privacy gate. |
| include/keepkey/emulator/libkkemu.h | Documents host-driven vs thread-driven modes; adds thread start/stop + flash lock APIs and clarifies display capture safety. |
| include/keepkey/board/util.h | Updates base_to_precision signature to size_t lengths. |
| include/keepkey/board/messages.h | Declares shared frame arena APIs for TX and large scratch. |
| include/keepkey/board/layout.h | Adds runtime icon support and progress trickle animation APIs. |
| include/keepkey/board/keepkey_board.h | Clarifies CRC32 contract and alignment/word_len semantics. |
| include/keepkey/board/font.h | Declares new length-bounded line measurement and paging helper APIs. |
| include/keepkey/board/draw.h | Declares RLE validation helper with strict well-formedness rules. |
| include/keepkey/board/confirm_sm.h | Clarifies constant-power fit semantics and simplifies review_immediate documentation. |
| include/keepkey/board/bsd_compat.h | New non-Apple emulator header providing strlcpy/strlcat prototypes. |
| fuzzer/firmware/CMakeLists.txt | Updates crypto include path for fuzzers. |
| docs/StorageVersionGate.md | New storage-version gate SOP doc and rationale. |
| docs/security/pin-kdf-v19-migration.md | New draft plan for PIN KDF v19 migration and release ordering constraints. |
| docs/security/anti-rollback-security-epoch-rfc.md | New RFC describing OTP-backed anti-rollback epochs and update state machine. |
| docs/Release.md | Strengthens release checklist (signature verification and storage-version gate reminders). |
| docs/README.md | Adds links to new storage gate and dice entropy docs. |
| docs/DiceEntropy.md | Updates dice entropy documentation to match removal of internal-entropy display. |
| docs/Build.md | Documents the two release products and Zcash privacy as part of full firmware only. |
| deps/crypto/CMakeLists.txt | Gates Pallas/Orchard crypto sources behind KK_ZCASH_PRIVACY; updates include dirs. |
| CMakeLists.txt | Bumps version to 7.15.0; introduces KK_ZCASH_PRIVACY selection; adds stack-usage reporting; emulator strlcpy prototypes include; AES table size selection notes. |
| cmake/toolchains/mingw-w64-x86_64.cmake | New MinGW-w64 cross toolchain for building Windows emulator DLL from non-Windows hosts. |
| cmake/caches/device.cmake | Forces integer-only snprintf variants on device builds to avoid linking float printf engine. |
| .gitmodules | Updates submodule refs/branches for device-protocol, trezor-firmware fork, and python-keepkey branch. |
| .gitignore | Ignores local build-btconly-check directory. |
Review details
- Files reviewed: 116/184 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
BitHighlander
merged commit Aug 27, 2026
9836fc0
into
release/7.14.3-bitcoin-only
26 of 36 checks passed
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review target
Fork-only stacked review PR for 7.15, based on the current
release/7.14.3-bitcoin-onlyreview head (64e9ce1c). This isolates the 7.15 product delta and preserves dependency order: review 7.14.3 first, then 7.15.Forward integration
The previously green 7.15 head (
c0546655) was merged forward with the new 7.14.3 head inbcadb714without rebasing or rewriting history.The merge had 36 conflicts where the release lines independently evolved CI, chain handlers, display flows, and tests. Conflicting hunks retain the already-audited 7.15 implementations; all non-conflicting current 7.14.2/7.14.3 changes were retained. The 7.15 version remains
7.15.0and storage remains V17.Included 7.15 delta
effdcd9eValidation baseline
c0546655was fully green: https://github.com/BitHighlander/keepkey-firmware/actions/runs/33038722649git diff --checkis cleanbcadb714is required before this review is considered convergedNo signing, tagging, publishing, release, upstream PR, or upstream comment is part of this fork review.