release: 7.14.2 security hotfix - #458
Conversation
BitHighlander
changed the title
ebb7fec to
c9dccf6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 105 out of 105 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
lib/firmware/fsm_msg_mayachain.h:157
denom_stris copied intoamount_str[32]here. A protocol-valid 68-character denomination cannot fit even before the numeric amount is added, and thebn_format_uint64()failure result is ignored, so the confirmation can receive an incomplete/invalid amount string. Size the final display buffer for the full formatted amount and fail closed when formatting fails, as the Binance path now does.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 120 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
Previously missed (4) — in code that hasn't changed since the last review.
lib/firmware/app_confirm.c:363
- The revised identity confirmation still does not bind the displayed protocol to the derived key:
cryptoIdentityFingerprint()hashesidentity->protocase-sensitively, while this function later uppercases it for the title. For example,sshandSSHshow the same protocol but derive different keys. Preserve the protocol bytes in the review, or canonicalize them consistently before both fingerprinting and display.
lib/firmware/fsm_msg_cosmos.h:427 - A malformed Bech32 receiver passes this preflight and triggers all IBC approval screens before
tendermint_signTxUpdateMsgIBCTransfer()finally rejects it. This contradicts the hotfix requirement that invalid signed values fail before approval. Apply the existing bounded Bech32 check here before opening any confirmation.
lib/firmware/fsm_msg_osmosis.h:682 - A malformed Bech32 receiver passes this required-text check, so the user approves the IBC details before
osmosis_signTxUpdateMsgIBCTransfer()rejects it. The PR promises invalid signed values fail before approval; invoke the existing bounded Bech32 validation during this preflight.
.github/workflows/ci.yml:787 - The PR description still names firmware
0a5c217and Python PR #219/36bc7923as the current evidence, but this workflow and gitlink now attest PR #221/ce5c1bb, and the current firmware head isfac6dc6. Consequently the documented exact-head runs and remaining release gates refer to different sources than CI will certify. Reconcile the PR metadata before treating these artifacts as release evidence.
KK_PYTHON_PR: https://github.com/keepkey/python-keepkey/pull/221
- app_confirm: page EVERY visual challenge through confirm_bytes(). The challenge is hashed into the signature on the non-SSH/GPG identity path, but a short one was strlcat'd into the body and drawn with "%s" -- layout text, not bytes -- so a control byte was invisible, leading spaces collapsed and a newline re-wrapped the screen. Two distinct signed challenges could produce an identical approval, and only challenges too long for the shared buffer got the byte-exact treatment. The metadata now always gets its own screen, so the title still names the protocol when there is no host or user. - app_confirm: stop uppercasing the identity protocol. cryptoIdentityFingerprint() hashes identity->proto exactly as sent, so "ssh" and "SSH" derive different keys while kk_strupr() rendered both as "SSH login to: ". Canonicalizing the fingerprint instead would strand every existing identity key, so the review shows the bytes that are actually hashed. - cosmos, osmosis: check the IBC receiver's bech32 form during preflight. The serializers refuse a malformed receiver, but only after every IBC approval has been taken. Its HRP belongs to the counterparty chain, so well-formedness is all that can be checked -- the same check the serializer makes, moved ahead of the confirmations.
|
All four suppressed findings from review 5038704932 are addressed in
Confirmed. Took the first of your two options. Canonicalizing before the fingerprint is not available to us: that hash derives every existing identity key, so changing its input would strand them. The review now shows the protocol bytes exactly as hashed.
Both confirmed and fixed:
Correct, and partly a consequence of the repin in
The only test evidence now claimed for this head is what I actually ran: 152/152 local unit tests (132 firmware, 16 board, 4 crypto) built through |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 120 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
lib/firmware/signtx_tendermint.c:298
- This update validates only
delegator_address; the uncheckedvalidator_addressis subsequently inserted into JSON with%s. This still permits malformed, wrong-network, or JSON-punctuation-bearing validator input in the signed document. Require a 20-byte Bech32<chainstr>valoperaddress here as well.
lib/firmware/signtx_tendermint.c:441 - The rewards path leaves
validator_addressunchecked while serializing it directly into JSON. As a result, the new safety checks still allow wrong-network/non-Bech32 values and JSON punctuation in the bytes that are signed. Validate it as a 20-byte Bech32<chainstr>valoperaddress before updating the hash.
The previous round gated every delegator/recipient address but left the
validator operators beside them unchecked, even though the same serializers
interpolate both into the signed document with a bare "%s". A wrong-network
operator, a plain account address where an operator belongs, or a value
carrying JSON punctuation still reached the signed bytes.
tendermint_validateValidatorAddress() builds the "<chain>valoper" HRP and
applies the same 20-byte account check. Wired into all four affected message
types on both signers:
- tendermint: MsgDelegate, MsgUndelegate, MsgBeginRedelegate (src and dst),
MsgWithdrawDelegationReward, with chainstr as the prefix
- osmosis: the same four, with osmo/tosmo
Only the Tendermint sites were reported; Osmosis has the identical defect in
the same four places and is fixed alongside.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 120 out of 120 changed files in this pull request and generated 2 comments.
Suppressed comments (5)
Previously missed (4) — in code that hasn't changed since the last review.
lib/firmware/fsm_msg_thorchain.h:160
- The recipient is confirmed before its network-specific Bech32 validation runs in
thorchain_signTxUpdateMsgSend(). A malformed or wrong-network address therefore receives a user approval and only then fails serialization, contrary to the fail-before-approval release invariant. Validate it against the selectedthor/tthorprefix before opening the screen.
lib/firmware/fsm_msg_mayachain.h:177 - The recipient is not checked until
mayachain_signTxUpdateMsgSend()runs after this approval. Consequently, malformed and wrong-network addresses are shown and approved before the transaction is rejected. Move themaya/smayaBech32 validation ahead of the confirmation to preserve the PR's fail-before-approval guarantee.
lib/firmware/fsm_msg_cosmos.h:172 - Cosmos recipient validation still occurs only in the serializer called after this screen. This lets invalid or non-
cosmosaddresses be displayed and approved before signing aborts, violating the PR's fail-before-approval invariant. Validate the address with thecosmosHRP before confirmation.
lib/firmware/fsm_msg_tendermint.h:181 - The destination is approved before
tendermint_signTxUpdateMsgSend()performs its Bech32/HRP/length validation. A malformed or wrong-chain address therefore reaches the consent screen and fails only afterward. Runtendermint_validateBech32Address(msg->send.to_address, msg->chain_name)before this confirmation so invalid signed values fail before approval.
lib/firmware/binance.c:126
- Accepting
bnbandtbnbindependently allows a transfer to mix network prefixes and does not bind either address to the signedchain_id. For example, a mainnet sign document can display atbnb...recipient even though the signature is domain-bound to mainnet. Select the allowed HRP from the transaction's chain ID (and reject unknown mappings), then require that HRP for every input and output.
Fail before approval, not after: - thorchain, mayachain, cosmos, tendermint: validate the recipient address ahead of the confirmation screen. Each serializer already refuses a malformed or wrong-network address, but it runs after the approval has been taken, so the owner approved a transfer that was then rejected. - ripple: ripple_formatAmount() now returns a status. It wrote "AMOUNT TOO LARGE TO DISPLAY" into a void function, so fsm_msgRippleSignTx() could not tell and signed the numeric amount behind that string. The handler also now refuses amounts above RIPPLE_MAX_DROPS, the bound ripple_serializeAmount() guarded with assert() -- which is compiled out of release builds, so beyond it the encoded amount differs from the one supplied. Bind what is displayed to what is signed: - binance: derive the one permitted address prefix from the session's chain_id instead of accepting bnb or tbnb per address. Taken independently, a single transfer could mix networks, and neither address was tied to the chain_id the signature is domain-separated by -- a mainnet envelope could display and sign a tbnb recipient. An unrecognised chain_id is refused. - app_confirm: refuse identity fields the renderer cannot represent unambiguously. proto, host, port and user are all hashed by cryptoIdentityFingerprint() but drawn as layout text, where a control byte is invisible and a leading space or newline changes the wrapping -- so two identities deriving different keys could present the same approval. They are URI components in practice, so they must now be 0x21..0x7E, the same range confirm_bytes() renders literally.
|
All five suppressed findings from review 5038966594 are fixed in Fail-before-approval on the recipient — All four confirmed. The validation added last round landed in the serializers, which run after the confirmation, so a malformed or wrong-network recipient was displayed, approved, and only then rejected. The same check now runs before the screen opens in each handler:
Confirmed, and this one was a genuine weakness in my own previous fix rather than a leftover. Accepting
An unrecognised chain ID has no prefix to derive, so initialization is refused rather than guessed. The prefix is cleared in Build clean; 152/152 unit tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 122 changed files in this pull request and generated 1 comment.
Suppressed comments (10)
Previously missed (10) — in code that hasn't changed since the last review.
lib/firmware/fsm_msg_osmosis.h:221
- These checks allow any safe-text delegator/validator strings through all three approval screens; the account and
valoperBech32 checks happen only afterward inosmosis_signTxUpdateMsgDelegate(). Validate both addresses against the session network before opening the first screen, otherwise an invalid operation is approved and only then refused.
lib/firmware/fsm_msg_osmosis.h:277 - As in the delegate branch, this only applies safe-text checks. The actual account/validator Bech32 validation runs after the user has approved the addresses and amount, contrary to the hotfix's fail-before-approval requirement. Perform the network-specific checks before the first confirmation.
lib/firmware/fsm_msg_osmosis.h:512 - The redelegation addresses are only checked as safe text here, while their account/validator HRPs and payload lengths are validated after all four approvals in
osmosis_signTxUpdateMsgRedelegate(). Move those checks before the first confirmation so malformed or wrong-network values cannot be approved first.
lib/firmware/fsm_msg_osmosis.h:579 - The reward addresses reach the approval screens after only safe-text checks; their account/validator Bech32 validation happens later in the serializer. This leaves the same approve-then-fail behavior that this hotfix explicitly removes elsewhere. Validate both addresses before the first confirmation.
lib/firmware/fsm_msg_osmosis.h:183 - The new validation still checks the recipient only as generic safe text. A malformed or wrong-network address is shown and approved here before
osmosis_signTxUpdateMsgSend()performs the Bech32/HRP check and rejects it. Move the network-specific address validation ahead of the first confirmation so invalid signed values fail before approval, as the Tendermint/THORChain paths now do.
lib/firmware/fsm_msg_osmosis.h:330 lp_add.senderis signed byosmosis_signTxUpdateMsgLPAdd(), but this only checks it as safe text and no subsequent screen displays it. The user therefore approves a transaction containing an undisclosed authority string. Either verify it against the expected signing account or add an exact sender-address confirmation before approving the LP operation.
lib/firmware/fsm_msg_osmosis.h:420lp_remove.senderis included verbatim in the signed JSON but receives only a safe-text check and is never shown. This contradicts the stated requirement that authorities be shown accurately or refused. Validate it as the expected signing account or explicitly confirm it before the LP screens.
lib/firmware/fsm_msg_osmosis.h:623- The swap
senderis signed verbatim but is neither network/account validated nor displayed; the added check only excludes unsafe text. A host can therefore obtain approval for a document containing an authority the device never showed. Bind it to the signer or add an exact sender confirmation.
lib/firmware/fsm_msg_osmosis.h:686 - The IBC
senderis signed but only receives a safe-text check and is omitted from the approval flow, even though the receiver and channel fields are shown. This leaves an undisclosed authority in the signed operation. Validate it against the signing account or display it explicitly before approval.
lib/firmware/fsm_msg_binance.h:139 binance_validateTransfer()does not perform the Bech32/network checks added in the serializer. Consequently the recipient is displayed and approved here beforebinance_signTxUpdateTransfer()rejects a malformed or wrong-network address. Run the full address validation before this confirmation to meet the hotfix's fail-before-approval guarantee.
Osmosis: the same fail-before-approval treatment the other Tendermint chains got last round, plus the authority gap beside it. - Recipients, delegators and validator operators are validated against this session's network before the first confirmation. They were only safe-text checked here; the bech32/HRP/length checks lived in the serializers, which run after every screen has been approved. - `sender` on the LP-add, LP-remove, swap and IBC messages is now bound to the account this session signs as. It is copied into the signed document verbatim and no screen ever displayed it, so the owner approved a document naming an authority the device never showed. Binding is stronger than displaying and costs no screen: there is exactly one account the session can act as, and a mismatch could not produce a valid transaction anyway. Binance: - Validate both transfer addresses against the session prefix before the confirmation. binance_validateTransfer() checks structure only, and the network check was in the serializer, after the approval. Ripple: - Require has_payment, has_amount, has_destination, and a destination that actually decodes, before either confirmation. An omitted field is not a zero field: the screens showed 0 XRP to an empty address while ripple_serialize() simply omitted what was missing, so the owner approved one transaction and the device signed another. ripple_serializeAddress() does enforce the 21-byte decode, but with assert() -- compiled out of release builds -- and only after both screens. - ripple_signTx() returns a status. It could return early leaving has_signature and has_serialized_tx false while the handler sent the response regardless, so a failure reached the host as an empty success.
|
All ten suppressed findings from review 5039140006 are fixed in Osmosis fail-before-approval — Confirmed across all five branches (send, delegate, undelegate, redelegate, rewards). Last round's fix reached the Tendermint, THORChain, MAYAChain and Cosmos handlers but not this one, so Osmosis still had safe-text checks in the handler and the real bech32/HRP/length checks in the serializer, after every screen. Added Undisclosed Confirmed for LP-add, LP-remove, swap and IBC. Each Took the "verify it against the expected signing account" option rather than adding a screen.
Confirmed. Build clean; 152/152 unit tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 122 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
lib/firmware/solana.c:806
- The fallback limit is not the transaction's default compute-unit limit. When
SetComputeUnitLimitis omitted, Solana derives the requested limit from the transaction's instructions (subject to the 1,400,000 cap); it does not request the cap unconditionally. For example, a one-instruction transfer with only a unit-price instruction can be charged on roughly 200,000 CUs while this code displays 1,400,000 CUs' worth as its “Maximum priority fee.” Derive the omitted limit according to runtime rules, or refuse/display only the unit price rather than presenting an inaccurate fee.
…g the cap When a transaction carries no SetComputeUnitLimit, the runtime requests 200,000 compute units per non-ComputeBudget instruction, capped at 1,400,000 -- it does not request the cap unconditionally. Assuming the cap could not understate the fee, but it overstated it badly: a transfer alongside a unit-price instruction is charged on 200,000 CUs and the device displayed seven times that as the "Maximum priority fee". Deriving the limit keeps the no-understatement property, because it is exactly what the runtime will charge, and makes the screen describe the transaction actually being signed. The test that pinned the old rule is updated. It now covers the realistic 1-instruction case and the cap, which SOL_MAX_INSTRUCTIONS (8) lets a transaction reach exactly with seven non-budget instructions -- so the clamp stays as defence rather than as a reachable path.
|
The suppressed finding from review 5039358111 ( You are right about the runtime rule: with no Your example checks out — a transfer plus a unit-price instruction at 2,000,000 micro-lamports/CU:
A 7× overstatement on the "Maximum priority fee" screen. One note on why this needed a test change rather than just a code change. The cap is reachable exactly, not exceeded: Build clean; 152/152 unit tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 122 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
lib/firmware/fsm_msg_ripple.h:105
- This new rejection path does not restore the home layout, unlike the other pre-confirmation failures in this handler. When PIN entry was required, the device can therefore send the fee error while leaving the PIN-entry UI on screen. Restore the terminal layout before returning.
…fee refusal - ripple_validateAddress() now requires the classic-account version byte, not just a 21-byte decode. ripple_serializeAddress() drops addr_raw[0] and signs only the 20 bytes after it, so an address with a valid checksum and a non-zero version displayed exactly as supplied while the signature committed to a different account -- the divergence this validator exists to prevent. - The fee refusal returned without layoutHome(), unlike every other pre-confirmation failure in the handler, so after PIN entry the error could be sent with the PIN UI still on screen.
|
The suppressed finding from review 5039531565 ( Correct — the fee refusal was the one pre-confirmation failure in (This one is mine: the amount and destination guards I added around it both call Build clean; 152/152 unit tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 122 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
lib/firmware/fsm_msg_tendermint.h:187
chain_nameis the coin/display name ("Cosmos"in existing requests), not the Bech32 HRP ("cosmos"; seeinclude/keepkey/firmware/coins.def:42). This makes validcosmos1...recipients fail here, and the same wrong argument reachestendermint_signTxUpdateMsgSend()at line 207, where it also derives the sender with an uppercase HRP. Usecoin->bech32_prefixfor both address operations and add a full valid-send regression.
- chain_name is not a bech32 prefix. coinByName() matches with strncasecmp(), so "Cosmos", "cosmos" and "COSMOS" all resolve to the same coin while only one is the HRP -- using chain_name for address work made correctness depend on the case the host happened to send. A request naming "Cosmos" would have had every valid cosmos1... recipient rejected by the check added last round, and TendermintGetAddress would derive "Cosmos1..." addresses no node accepts. Both the handler's validation and the serializer's sender derivation now take coin->bech32_prefix. Gated on the prefix STRING, not coin->has_bech32_prefix: in coins.def the tendermint family carries a populated prefix behind a false flag (Cosmos is `false, "cosmos"`), so requiring the flag would refuse every Cosmos transaction. The new regression covers this. - The IBC sender is bound to the account this session signs as. tendermint_signTxUpdateMsgIBCTransfer() derived from_address and then discarded it, writing the host's sender into the signed JSON instead, so an arbitrary sender produced a signed message no key could authorize. The serializer now requires the two to match, and the Cosmos handler refuses a mismatch before any screen via tendermint_addressIsSigner().
|
The suppressed finding from review 5039677780 ( Confirmed. Fixed at all three places — the recipient validation, the One correction worth recording, because it nearly turned into a worse bug than the one being fixed. The obvious guard is Gating on the flag would have refused every Cosmos transaction outright. The handlers gate on the prefix string being non-empty instead. Added Build clean; 153/153 unit tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 122 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
lib/firmware/fsm_msg_crypto.h:70
- This opens the identity/key confirmation before validating either
has_identityor the host-selected curve. A missing identity or unsupported curve can therefore receive approval screens and only fail afterward, contrary to this hotfix's pre-approval validation invariant. Validate the fingerprintable identity and curve first, then prompt.
…roval - thorchain, mayachain: the outer transaction memo is reviewed whenever it is present, including when the deposit carries its own. These are two different strings in the signed document, not one superseding the other: signTxInit() hashes sign_tx->memo into the StdSignDoc "memo" field and the MsgDeposit value hashes deposit.memo separately. Gating the outer review on !deposit.has_memo let a host show a benign deposit memo while a different outer memo was signed unseen. - SignIdentity: check has_identity and the curve before the confirmation. The identity check sat after it and the curve was not validated until fsm_getDerivedNode(), so a request with no identity or an unsupported curve collected a full approval -- and, for the curve, a PIN entry -- before failing. The curve selects the key, so it belongs on the screen's side of the line.
|
The suppressed finding from review 5039874083 ( Confirmed on both counts. Both checks now run first. The curve matters especially: it selects the key the signature is made with, so it belongs on the screen's side of the line rather than after it. Build clean; 153/153 unit tests pass. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 122 out of 122 changed files in this pull request and generated 3 comments.
Suppressed comments (5)
Previously missed (5) — in code that hasn't changed since the last review.
lib/firmware/ethereum_contracts/saproxy.c:98
- Format the withdrawal amount before showing the salary-ID confirmation. With the current 41-byte buffer, a valid large
uint256amount makes the secondsa_formatUint256()fail only after the user approved the first screen; the dispatcher then reports that validation failure as a user cancellation. Pre-format both values so an unrenderable call fails before any approval is requested.
lib/firmware/eos.c:492 - The newly saved
unknown_commonincludes the action's authorization actor/permission list, but those values are only compared and hashed; the eventual approval shows contract, action name, data length, and data fingerprint. An Advanced Mode user can therefore approve an unknown action without seeing the authorities embedded in the signed transaction. Render everyactor@permissionentry (or include them in an exact reviewed fingerprint) before accepting the action.
lib/firmware/fsm.c:325 fsm_abort_workflows()does not restore the home layout, and several newly covered sessions (Binance, Tendermint, Osmosis, THORChain, MAYAChain, EOS, and Nano) have abort functions that only clear state. Clearing a session while one of them waits for an ACK therefore leaves the canceled transaction approval on the OLED. Return to the home screen after aborting, asInitializealready does.
lib/firmware/ethereum_contracts/thortx.c:202- The amount is still formatted only after the router, vault, and (for unknown assets) token-address screens have been approved. Large valid amounts can make either formatting branch return false here, and the Ethereum dispatcher reports that late validation failure as
ActionCancelled; the expiry-width and memo-parse checks below have the same partial-approval behavior. Complete all non-interactive validation and formatting before the first confirmation screen.
lib/firmware/fsm_msg_common.h:566 - Cancel now clears all signing state, but it still leaves the previous transaction screen visible for workflows whose abort routine has no layout side effect (for example Binance or THORChain). That stale approval screen no longer corresponds to any active operation. Call
layoutHome()after the centralized abort before sending the failure.
…il early Bind the authority to the key that signs: - thorchain, mayachain: MsgDeposit's `signer` must be the account this session derives, not merely a well-formed address on the right network. It is serialized verbatim as the message authority, and the confirmation labels it as though it were a destination, so a valid-but-foreign signer produced a signed document the device cannot authorize and the screen did not give it away. - binance: a transfer's input is its authority and no screen displays it, so network-correctness was not enough. It is now required to equal the session address. The output stays a plain address check -- it is the recipient, and it is shown. Disclose what is signed: - eos: show every actor@permission on an unknown action. The authorization list is compared across chunks and hashed into the preimage, but the approval named only the contract, action, byte count and data fingerprint -- omitting the one part of an opaque action that says how much it may do. Fail before approval: - thortx: resolve the asset, render the amount and validate the expiry before the first screen. They ran after the router, vault and asset screens, and ethereum.c reports a late false return as ActionCancelled -- so a large but valid amount told the owner they had cancelled what they had just approved. - saproxy: format both values before either screen, for the same reason. Don't leave a cancelled screen up: - Cancel and ClearSession now call layoutHome() after aborting. The Binance, Tendermint, Osmosis, THORChain, MAYAChain, EOS and Nano abort routines only clear state, so the cancelled transaction's approval screen stayed on the OLED. Placed at these two call sites rather than inside fsm_abort_workflows(), which toggle_screensaver() also calls immediately before drawing the screensaver.
|
All five suppressed findings from review 5040032907 are fixed in
Both confirmed, and the consequence you identified is the sharp part:
Confirmed.
Confirmed for both I put Build clean; 153/153 unit tests pass. |
Three conflicts, plus one semantic clash the auto-merge hid. .github/workflows/ci.yml -- took develop's pipeline (PR #474 cut it from ~7:20 to ~2:00 and folded check-submodules, build-emulator, unit-tests and python-integration-tests into build-and-test) and re-applied what this branch adds on top: - permissions: contents: read at the workflow level. develop has no permissions block, so code-scanning rule actions/missing-workflow-permissions had one alert open per job. - generate-test-report restored as its OWN job rather than folded into build-and-test. develop's fold is correct for develop's report script, which only needs directories that job already has. This branch's generate-test-report.py also consumes test-reports/dylib-junit.xml and the ARM manifest -- produced by other jobs, on other runners -- and fail()s hard when either is missing. Folded, the report could never succeed. - the report's provenance env (both source SHAs, run URL, both PR URLs). develop passes only KK_BUILD_LABEL, which this branch's script does not read; the fields it does read are release gates for 7.14.2. - the report artifact is a directory again. The script emits test-report/test-report.pdf beside test-report-manifest.json and test-report.pdf.sha256; develop uploaded a bare test-report.pdf, which would have found no files. - release-evidence-gate, rewired to develop's folded job names, so a release still has one required check that every evidence-producing job succeeded. .gitleaks.toml -- union. Kept develop's header and its docs-SHA allowlist, which is broader than and subsumes this branch's python-keepkey-pin rule, plus this branch's U2F/vendored-deps allowlist and the note recording why no first-party test tree is exempted. scripts/emulator/python-keepkey.Dockerfile -- took develop's parameterised deps-stage version wholesale. scripts/emulator/Dockerfile did NOT conflict: only this branch had changed it (base -> digest), so git kept the digest silently. That defeats develop's GHCR mirror, which tags the mirrored image as kktech/firmware:v15 -- a digest FROM ignores that tag and pulls from Docker Hub on the heaviest job in the run. Parameterised it the way python-keepkey.Dockerfile already is, defaulting to the digest so a release build still names an immutable base, with CI passing the resolved mirror. kktech/firmware:v15 resolves to exactly that digest today. Verified: emulator image builds both with the default and with an explicit --build-arg; 153/153 unit tests pass; gitleaks 8.30.1 finds no leaks with the merged config; clang-format clean.
Both were mine, and both were invisible to the emulator build I had been verifying against. ripple.c: add the memzero.h include. ripple_validateAddress() calls memzero() and the header was never included. The emulator build (clang, permissive) accepted the implicit declaration; the ARM cross-compile (-Werror=implicit-function-declaration) and the macOS dylib build (clang, C99) both refused it, so build-arm-firmware and python-dylib-tests have been red since 5c14acc. Verified by running the ARM cross-compile from ci.yml locally: clean, no warnings. The fsm_msg_*.h files that also call memzero() are #included into fsm.c, which has the header -- ripple.c was the only real translation unit affected. fsm_msg_ethereum.h: build the GetAddress response in locals and commit it to `resp` only after the confirmation. `resp` aliases fsm.c's single msg_resp buffer, and confirm_* runs a message loop -- every DebugLink request the emulator harness makes while a screen is up is dispatched from inside it, and those handlers RESP_INIT the same buffer. Staging address_str before the screen left it live across arbitrary foreign writes, which is how EthereumAddress.address_str reached the host as undecodable bytes and broke test_ethereum_show_address. Restoring the address QR layout is what exposed it. fsm_msgNanoGetAddress() already builds into a local and assigns after its confirm; this handler was the one that staged first.
251f7a3 added two consumers of arm-build-manifest.json -- validate_arm_manifest() in generate-test-report.py and the presign verification in release.yml -- and no producer. generate-test-report has failed on this branch ever since, first masked by build-arm-firmware failing for other reasons. build-arm-firmware now writes it beside the binaries: firmware_sha, python_sha, and a sha256 per .bin/.elf. That job is the only place holding the artifacts and both source SHAs at once. firmware_sha uses the same expression as this job's checkout and the report's KK_FIRMWARE_SHA, so the three cannot drift. Verified against a real local ARM build: validate_arm_manifest() accepts the generated manifest across all 23 artifacts and still refuses a mismatched firmware SHA.
7.14.2 security hotfix
This release makes device approval match the exact operation being signed. It closes the display, cancellation, bounds-checking, validation, and secret-cleanup issues found during the 7.14.2 review without adding product features.
Current review heads:
623d7d5accc38e4ac864834bf552e99c2d3e375ece5c1bb09ed2f9504df6c93fff103af208e237d4(merged; python-keepkeymasterhead, and the commitdeps/python-keepkeyis pinned to)Python #219 is merged but is no longer what this PR attests: the submodule pin
92e8745was #221's first commit, so the tree predated #221's7c04903 "test(solana): match AdvancedMode refusal". Repinned to #221's mergecommit and
KK_PYTHON_PRupdated to match.What changed
0xeeee..eeeetoken lookup is strictly chain-scoped. Standard transfer, approve, andOutputAddressType_TRANSFERreview showUnknown token valueoutside chain 1.Chain-ID finding provenance
The one-byte
TokenType.chain_idfield predates this release line (December 2022). A widening fix existed on another development line in May 2026 but was not included here. The prior candidate commit251f7a3fadded a transfer-formatting lookup that inherited the narrowing, increasing the affected surface without creating the underlying defect.This head widens the token and legacy-coin lookup boundaries to
uint32_t, restricts the legacy ticker fallback to chain 1, closes the residual cross-chain native pseudo-address exception, and adds firmware plus emulator OLED regressions proving chain 257 cannot resolve chain-1 metadata.Exact-head verification in progress
The previous 616-test/619-frame report, binary hash, and run links belong to superseded heads and are not evidence for this candidate.
33033473265,33033470261) were for0a5c21729and are superseded; exact-head CI has not yet been run or reconciled for623d7d5ac.scripts/emulator/Dockerfile.Remaining release gates
Approve and merge Python firmware: Stellar support #221, verify its durable master tree, and repin this PR to that durable commit.Done: pinned toce5c1bb, which is python-keepkeymasterhead.This candidate is ready for independent review once exact-head evidence is reconciled. It is not authorization to merge, sign, or tag.