7.15 release: consolidated audit stack - #364
Conversation
Sets the 7.15.0 project version and the build-time plumbing the rest of the release sits on: - KK_BITCOIN_ONLY option plus the derived KK_ZCASH_PRIVACY selection, so the product is two images (regular, bitcoin-only) rather than three. - Device builds route snprintf/vsnprintf to newlib's integer-only engine and emit per-function stack-usage data for the SRAM budget gate. - tools/check_sram_budget.py + tools/sram-budgets.json enforce the stack frame margin that the RC7 boot fault exposed. - CI: variant matrix across ARM/emulator builds and unit tests, a crypto job running the pinned fork's suites under Valgrind and ASan/UBSan, release invariant checks, action SHA pinning and digest-pinned builder images. - Release workflow supports RC tags and selective submodule checkout. - Dependency pins move to the reviewed heads: trezor-firmware fork, and the device-protocol / python-keepkey commits carrying the 7.15 messages. - Include paths follow the crypto fork's layout (trezor-firmware/crypto). Feature code and its per-feature CI gates land in the branches stacked on top of this one; nothing here compiles new firmware sources.
Display: - Long bech32/bech32m addresses were cut at the first 42 characters on the verify screen, so the QR code and the text disagreed about where funds were going. Addresses now wrap across the full screen, and Zcash unified addresses get a dedicated text-first confirmation layout. - Font, draw and layout helpers gained the measurement and wrapping support those screens need. ROM: - Device snprintf/vsnprintf are redirected to newlib's integer-only engine (see cmake/caches/device.cmake), which removes the float formatting path, soft-double libgcc and its malloc dependency from the image. Percent and amount rendering that used %f is reworked to integer formatting, so THORChain withdraw percentages and send amounts still display exactly as before — Gate-3 frames under docs/security/evidence/rom-printf-integer-percent. - keepkey.ld asserts the 16 KiB stack reserve the SRAM budget gate measures.
msg_write() encoded outbound frames into a MAX_FRAME_SIZE TrezorFrameBuffer declared as an automatic — a ~12.4 KB stack frame. With the Orchard engine linked in, static SRAM left an ~11 KB gap and the device hard-faulted on boot during RC7 bring-up. Inbound reassembly, outbound encode and the recovery-cipher wordlist scratch are mutually exclusive on a strictly cooperative single-threaded transport (there is no USB ISR; usbd_poll runs only from explicit usbPoll call sites), so they now share one static union. Acquiring the arena for TX or scratch drops any partially reassembled inbound frame; only a host that pipelines a second request before reading the first response can observe it, and it gets a Failure on its next continuation frame rather than silent corruption. The reassembly state moves to file scope so arena acquisition can invalidate it.
|
CI queue note: this head was explicitly dispatched at https://github.com/BitHighlander/keepkey-firmware/actions/runs/31128231885 after the superseded 17-PR stack saturated the Actions queue. GitHub currently shows the jobs queued before runner assignment. The complete aggregate independently passed the CI-equivalent container suite locally: 405 firmware, 2 board, 18 crypto, and 7 Pallas constant-time tests, plus actionlint, gitleaks, the Pallas API-boundary gate, and git diff --check. |
Two problems with how this workflow gated the 7.15 audit stack. The pull_request trigger listed only master/develop/alpha, so every PR in the train except the first -- each based on the previous release branch -- matched nothing and reported "no checks" in the GitHub UI. The push trigger already covers 'release/**', so those branches were being built and were red; the PRs just never showed it. A reviewer saw a clean PR sitting on a failing build. Add 'release/**' to the pull_request branches so the checks attach to the PR. Second, the RC18 invariant forbidding libc random() in lib/rand/rng.c was introduced here, in the foundation slice, but the emulatorRandom() change that satisfies it arrives three slices later with the rest of the RNG work. The result was that this branch and the clear-signing branch failed check-submodules and skipped every build and test job behind it -- nothing was compiled or run on either. Move the invariant to the commit that makes it true; a gate that fails for code its own PR does not contain teaches reviewers to ignore the gate. No invariant is lost: it comes back, unchanged, in the RNG slice.
Adds the signed-metadata engine: a host may supply a description of what an EVM transaction does (contract alias, method, argument labels), and the device displays it only if the blob carries a signature from a trust anchor it already holds. Unsigned or wrongly signed metadata is ignored — it can never turn a blind-signing prompt into a friendly one. - Anchor verification over secp256k1 with the schema bound to the request, so a signature captured from one transaction cannot dress up another. - Runtime signer table only; nothing is persisted to public flash. A rogue entry surviving a reboot would suppress the raw-data screen, which is exactly the failure this design refuses. - tiny-json gains the bounds needed to parse metadata safely on device. - unittests/firmware/signed_metadata.cpp covers the accept/reject matrix, including malformed blobs and signature/schema mismatches. Message plumbing and the Ethereum call sites arrive in the clear-sign EVM branch; this commit is the engine and its tests only.
…hemas Solana signing moves from "confirm this blob" to describing the actual instructions, using schemas that are signed once and reused: - v0 (versioned) transactions and address-table lookups are parsed, and the instruction set is rendered per program: transfers, token transfers, memo bodies and offchain messages. - A schema describes a program's instruction layout (discriminator, argument types and labels) and carries a signature with no transaction hash bound to it, so one blob covers every call of that shape instead of needing a fresh signature per transaction. - x402 payment requests are clear-signed through the same path: the payment amount, asset and recipient are shown from schema-described fields rather than trusting host-supplied text. - Anything without a verified schema still reaches the raw-data screen. - unittests/firmware/solana.cpp covers v0 parsing, schema decode, argument rendering and the reject paths for malformed or unsigned schemas.
Uses the signed-metadata engine on the Ethereum path so a user sees what a contract call actually does instead of a hex blob: - EthereumTxMetadata carries the signed description and EthereumMetadataAck acknowledges it; LoadClearsignSigner installs a trust anchor for the current session only. Metadata that fails verification is dropped and the transaction falls back to raw display — it never degrades to a friendlier screen. - EIP-712 typed data: domain/primary-type handling is corrected, the policy gate decides when a typed hash may be signed at all, and the recursion and length limits are enforced rather than assumed. - EIP-7702 authorization lists are recognised and shown. - ERC-20 and the bundled contract decoders (THORChain router, liquidity and swap proxies) check the selector against the argument shape before claiming to know what a call means. - A factory reset clears loaded signers. - unittests/firmware/eip712.cpp and the extended ethereum.cpp cover the typed data limits, selector mismatches and the metadata accept/reject paths.
Adds messages 1700-1703 so a KeepKey can issue the schema signatures the clear-sign path verifies, instead of every schema having to come from an offline signing ceremony. - ClearsignAttestorGetPublicKey exports the attestor key; ClearsignAttestorSign signs a schema after showing the user what they are attesting to — program identity, discriminator, and each argument label with its type and width. - Self-service attestation is gated behind advanced mode, so a default device cannot be talked into minting trust for an attacker's schema. - The signed variant ships the built-in anchor table, so schemas issued this way verify on a stock device with nothing loaded. - scripts/emulator/capture-clearsign-attestor.py reproduces the OLED sequence, and docs/security/evidence/7.15.0-rc21-clearsign-attestor holds the captured frames including the 44-character program id and 16-character label bounds.
Storage: - The PIN key-derivation function is versioned so its cost can be raised without breaking existing wallets, with the migration path documented in docs/security/pin-kdf-v19-migration.md. - A wallet created by bitcoin-only firmware is stamped, and multi-chain firmware refuses to touch it (storage_isBitcoinOnlyLocked) rather than silently operating on a wallet whose owner chose a reduced attack surface. - Storage layout reserves the clear-sign identity block and zeroes it on read and on wipe, so nothing there can outlive a factory reset. - Orchard key derivation and seed fingerprinting are reachable through storage with progress reporting, guarded by the privacy build flag. - docs/security/anti-rollback-security-epoch-rfc.md records the proposed security-epoch scheme; it is a design note, nothing here implements it. BIP-85: - GetBip85Mnemonic derives a child mnemonic and displays it under constant power, with PIN and initialization checks. Always available — it is a seed derivation, not a coin engine. Recovery and authenticator: - The cipher-recovery wordlist permutation now borrows the shared frame arena instead of another multi-kilobyte stack buffer. - wipeAuthData reports failure instead of always claiming success, a cancelled authenticator action is distinguishable from an error, and a cancelled passphrase prompt aborts the request instead of continuing without one.
BitHighlander
changed the title
BitHighlander
changed the title
Every release through v7.14.1 published firmware.keepkey.bin. The two-variant matrix started stamping matrix.variant into the filenames, so the regular build would have shipped as firmware.keepkey.v7.15.0-full.bin -- a rename that gains nothing and breaks the name users, docs and download scripts already know. The job names already had this right: 'label' is empty for the regular build and only bitcoin-only is annotated. Do the same for filenames by adding a 'suffix' key alongside it, empty for the regular build and "-bitcoin-only" for the other. matrix.variant stays what it is -- the build selector, and the provenance value. Only the unusual variant is qualified now: firmware.keepkey.v7.15.0.bin HASHES.txt PROVENANCE.txt firmware.keepkey.v7.15.0-bitcoin-only.bin HASHES-bitcoin-only.txt ... Also fixes a bug this rename would otherwise have introduced: create-release collected hash and provenance files with 'HASHES-*.txt' / 'PROVENANCE-*.txt', which stop matching once the regular variant's files lose the dash -- the regular build's hash manifest would have been dropped from the release without failing the job. Globs are now HASHES*.txt / PROVENANCE*.txt. Internal-only artifact names (emu-image-<variant>, unit-test-results-<variant>) are untouched; they never leave CI and renaming them would just churn their download-artifact consumers.
The report is rendered from the merged JUnit (Python integration + the four native firmware unit XMLs) but --validate-junit was handed the Python JUnit alone. Any catalog entry naming a native unit test therefore resolved to "missing" and failed the gate, so no native test could ever be catalogued -- the firmware-unit XMLs were merged in and then unusable, which is half the reason 432 native tests were invisible to the release report. Validate against the merged file. The requirement that canonical Python evidence exist and be non-empty is enforced earlier, before the merge, so this weakens nothing. Verified locally against the 7.15.0 RC artifacts from run 31284108490: the trigger renders 325 catalogued tests and exits 0, and injecting a failure into Storage::PinKdfV16RewrapsToV19AfterCorrectPin makes it exit non-zero with "report catalog validation failed". Pairs with python-keepkey "report: catalog the 7.15 seed-generation evidence and state the report's scope"; the pin must move for section K to appear.
test_dylib_confirm_flow.py contains two tests and neither has ever run in CI. The main python job skips the file (it needs KK_TRANSPORT=dylib), and the dylib job invoked test_dylib_screenshot.py only, so both were invisible -- the skip reasons appear in the JUnit but no gate reads them. They are not equivalent, and treating them as one file is what kept both out. test_features_round_trip is the pure Initialize -> Features path with no confirm at all; its own docstring says it should always pass. That one runs now. test_load_device_with_auto_confirm stays skipped, and deliberately: it is blocked on an open firmware defect, not on test plumbing. confirm_helper busy-loops on a ButtonAck the dylib consumed but never delivered, which is the same path keepkey-vault's wipe_device hangs on, and no pytest-timeout method can interrupt a C-level kkemu_poll(). Adding the file wholesale would convert a silent gap into a job that burns its 25-minute timeout. Named explicitly rather than by file so the distinction is visible at the call site instead of living only in a skip decorator.
…iant 27970b0 gave the regular variant an empty suffix so its published assets keep their plain names. Three of the four renames in "Rename artifacts" also insert the version, so they stay distinct. The fourth does not: mv HASHES.txt "HASHES${SUFFIX}.txt" With SUFFIX empty that is `mv HASHES.txt HASHES.txt`, and GNU coreutils does not treat a same-file move as a no-op -- it errors "are the same file" and exits 1. The step runs under the default `bash -e`, so the regular firmware's release job dies there, after the build and the hashing have already succeeded. Only the bitcoin-only leg would publish. Nothing caught this because release.yml runs on tag push only: the branch's 28/28 green CI never executes it, and the last green release run (rc26, 2026-08-04) predates the suffix change -- it still renamed to "HASHES-${VARIANT}.txt", which is always distinct. The first thing to exercise this path would have been the v7.15.0 tag itself. Skip the move when there is nothing to rename. The regular variant's intended output name is plain HASHES.txt, which is what it already has. Verified by running the step's script under `bash -e` for both matrix legs: empty suffix exits 0 leaving HASHES.txt, "-bitcoin-only" exits 0 leaving HASHES-bitcoin-only.txt.
SOP review — 8 reviewers + adversarial verificationCadence step 3. Eight reviewers (one per commit group, xhigh effort), each finding then attacked by an independent agent instructed to refute it and to default to refuted when it could not confirm from source. 17 agents, 734 tool calls. 28 raised → 7 confirmed, 2 refuted, 19 unverified. The 19 are medium/low or past the 3-per-group verify cap — they were not dismissed, just not adversarially checked. They are listed at the bottom so nothing is silently dropped. Two were then re-verified by hand and both stand. Fixes for the release-blocking one are in #365. Hand-verified independentlyOrchard amount clipping — confirmed, with the mechanism.
Confirmed1. [CRITICAL] Orchard output confirm screen overflows the OLED — the shielded amount is never drawn
The only on-device gate that shows a shielded output's value builds a 5-line body for a screen that renders exactly 3 lines, so the "Amount:" line and the last ~31 characters of the Unified Address are silently dropped and never seen by the user. Suggested fix: Do not pack the address and the amount into one confirm() body. Show the amount on its own screen (or first), and render the 106-char UA through a paginated address layout such as the existing confirm_zcash_address()/layout_zcash_address_text_notification path used by fsm_msgZcashDisplayAddress, which is built for a full-length UA. Add a compile-time or runtime assertion that any confirm body destined for layout_standard_notification fits within three rendered lines. 2. [HIGH] A verified Solana schema makes a MALFORMED transaction signable and skips the signer-membership check
The new Suggested fix: Reject SOL_TX_REVIEW_MALFORMED before the schema dispatch, e.g. 3. [HIGH] Unbounded index in fsm_msgZcashTransparentInput writes past transparent_inputs[]
The handler only checks msg->index == current_transparent_input and never checks msg->index < ZCASH_MAX_TRANSPARENT_INPUTS, so once the declared input count has been consumed the counter keeps advancing and indexes past the end of the fixed 8-element array. Suggested fix: Reject the message unless 4. [HIGH→MEDIUM] Clear-sign tx_hash binding can be silently disabled mid-signing (ClearSession / LoadClearsignSigner)
fsm_msgEthereumTxMetadata refuses to run while an Ethereum signing session is open precisely because clearing the metadata mid-flight drops the tx<->metadata binding, but the sibling handler fsm_msgLoadClearsignSigner (and the unauthenticated ClearSession path) also clear that binding and have no such guard, so signed_metadata_enforce() falls through to 'not gated' and the device emits a signature over calldata the user never saw decoded. Suggested fix: Make the binding survive a clear, or refuse to clear while it is live. Minimum: add the same 5. [HIGH→MEDIUM] TRC-20 confirmation screen silently drops the destination address for large token amounts
The TRC-20 transfer approval packs a host-controlled uint256 base-unit amount and the 34-character recipient address into one unpaged confirm() body, so a large amount pushes the recipient past the OLED's three body rows and it is never drawn. Suggested fix: Split the recipient onto its own renderer-measured screen and page both values, e.g. confirm_bytes(..., "Send To", to_str, strlen(to_str)) and confirm_bytes(..., "Token Amount", amount_str, strlen(amount_str)), the same pattern fsm_msg_osmosis.h uses for "Send Amount"/"Send To". Never place a host-length-controlled field and a security-material fixed field in one unpaged confirm() body. 6. [HIGH→MEDIUM] BIP-341 hash accumulators are conditionally initialised but unconditionally used, so a host-set
|
Release evidence: complete skip inventory and waiversThe run's 12 skips, enumerated from the JUnit rather than summarised, so none of them passes silently. There are no others — 1237 testcases, 0 failures, 0 errors. Waived for this release (3)Uniswap liquidity, Ethereum section 23/26 — this is why the header is not green.
Cause is disclosed and credible: an approve to an unknown (non-registry) token contract cannot complete against kkemu. Waived on condition that all three are walked on real hardware during the RC device round. If that walk does not happen, this waiver is void. Not waived — carried as open defects (3)
Structural, not a gap (6)The four On reading this reportAbsence of a feature from |
The SRAM budget gate has always enforced correctly -- verified against the shipped 7.15.0 RC ELF, where the reserve measures 17,940 B against a 16,384 B budget, and all three of its failure modes exit 1: reserve breached, largest frame eating the margin, and an empty .su archive (it refuses to pass the frame-margin check without stack-usage data rather than treating "no records" as "largest frame = 0"). But its output only ever existed in a job log nobody opens. The frame-arena fix -- the RC8 msg_write boot fault, the single largest SRAM change in this release -- therefore had no reviewable evidence anywhere, and an RC audit recorded it as having zero coverage. It did not; nothing surfaced it. Mirror the report into the run summary, per variant. The status handling is deliberate. `tee` would otherwise swallow a non-zero exit, so pipefail is on; and the status is captured rather than allowed to abort the step under `set -e`, because a FAILING gate is exactly when the numbers need to reach the summary. The step re-exits with the captured status, so a breach still fails the build. Verified by extracting the step's script and running it against stubs for both outcomes: pass exits 0, breach exits 1 and the summary still contains the ::error:: line. shellcheck clean.
Follow-up on the CRITICAL: "Orchard" is a misleading name here — Ironwood shares the screenRaised because the live path is Ironwood, not Orchard. It does not narrow the finding; it widens it.
bool cmx_ok = zcash_signing.is_ironwood
? zcash_ironwood_compute_cmx_with_progress(...)
: zcash_orchard_compute_cmx_with_progress(...);Everything after that — And there is no compensating screen. The summary confirm shows an uint64_t total = msg->has_total_amount ? msg->total_amount : 0;The code says so itself at
The gate it delegates Orchard output values to is the per-output confirm — the screen whose Measured, not estimatedA real Orchard unified address is 106 characters — confirmed from the captured Why it was never seenThe RC run captured 1037 OLED frames and not one matches BitHighlander/python-keepkey#30 now requests frames on the one catalogued test that exercises a real shielded output, so the fix can be checked against a captured frame rather than an assertion. Suggested fix unchanged: amount on its own screen, full UA through the existing paged |
…screen
The per-output shielded confirm built
"Send shielded ZEC?\n<unified address>\nAmount: %s"
for a three-row body. A unified address is 106 characters -- three full rows
on its own, as the captured display-address frame shows (38 + 38 + 30). The
standard notification does not scroll or paginate: draw_string stops emitting
as soon as a character will not fit, with no indication. So the screen
rendered the question plus the first 76 characters of the address, and
silently dropped the remaining 30 along with the entire amount line.
This is the gate that was supposed to show the value. total_amount on the
summary prompt is taken straight from the host message, and the contract
comment in the signing path says so outright -- it calls total_amount "a
summary prompt" and states that Orchard output recipients and values "have
their own verification gates". This confirm is that gate. The device does
bind value to cmx, so a host cannot present an inconsistent note; the defect
is that the user approved a value they were never shown.
Both pools are affected. is_ironwood selects only which compute_cmx runs;
the address rendering and the confirm are the shared tail of the same
function, so Ironwood and Orchard render the identical screen.
Amount first on a body that cannot overflow, then the full address through
layout_zcash_address_text_notification -- the layout built for a 106-char UA
and already used by the display-address flow.
VERIFICATION LIMITS, stated plainly:
- The overflow is established from the code and from a measured 106-char
UA in a real captured frame, not from a rendering of this screen.
- Nothing in CI executes it. ZcashSignPCZT is never sent to a device
anywhere in the suite -- every test in test_msg_zcash_sign_pczt drives a
ScriptedTransport with canned responses. The device-driven Zcash tests
cover transparent signing, display-address, FVK and seed fingerprint.
- So this compiles in CI and is not otherwise exercised. It MUST be walked
on hardware during the RC round, and both screens photographed.
- The flow now emits two ButtonRequests per shielded output instead of
one. Hosts that loop on ButtonRequest/ButtonAck are unaffected; anything
counting exact requests needs updating.
Moves deps/python-keepkey to 3bbf996, which adds the first test that actually sends ZcashSignPCZT to a device. Until now every PCZT test drove a scripted transport with canned responses, so the on-device shielded path -- including the per-output confirm this branch fixes -- was never executed by anything. The new test is a real regression test for that fix, not a companion to it: run against the shipped 7.15.0 RC emulator it fails with "expected 2 ConfirmOutput screens, got 1". This pin is what lets CI run it against the fixed firmware. Also brings the report catalog changes: native firmware unit tests can now be referenced at all, the header states how much of the run the catalog covers, and the shielded confirm screens are finally requested as screenshots. PRACTICE PIN -- 3bbf996 is on an unmerged fork branch (BitHighlander/python-keepkey#30, stacked on keepkey/python-keepkey#211). Re-pin to a merged master SHA before tagging v7.15.0, along with device-protocol and trezor-firmware.
…f it
Two defects, both found by the integration job failing on this branch.
1. A failing test run threw away everything it had produced.
The step comments say "always extract reports", and the upload steps carry
if: always(). Neither helped, because the step runs under the default
`bash -e` and captured exit codes with
docker compose up --exit-code-from python-keepkey python-keepkey; PY_RC=$?
A bare failing command aborts immediately under -e, so the abort happened
before the assignment and before the docker cp block. Nothing was ever
copied out of the container, and if: always() had nothing to upload.
The run that exposed this captured four OLED frames of the Zcash shielded
confirm -- the first shielded frames this project has ever produced -- and
uploaded none of them. Its JUnit and PDF went the same way. Evidence
disappearing exactly when a run fails is the worst possible time for it.
`|| RC=$?` keeps the assignment and the extraction on the path while still
failing the step afterwards. Verified by running the step's own script with
a deliberately failing container: extraction is reached, RCs are captured,
and the step still exits 1. With the semicolon form the extraction line
never prints.
2. The in-container catalog validation could not see native unit tests.
python-keepkey-tests.sh validated against the Python JUnit alone, so every
catalog entry naming a native firmware unit test resolved to "missing" --
the same defect already fixed in scripts/generate-test-report.py, in a
second call site. The test-reports volume is shared rw with the
firmware-unit container, which runs first, so those XMLs are already
present; merge them before validating and before rendering.
If they are absent this falls back to Python-only and native entries then
fail as "missing" — it still fails closed rather than quietly passing.
Verified against the real artifacts from the failing run: merging the
native XMLs resolves K3-K11, leaving only the Z22-Z25 entries that the
older Python JUnit predates.
fsm_msgZcashTransparentInput checked that msg->index matched current_transparent_input, but never that either was inside transparent_inputs[ZCASH_MAX_TRANSPARENT_INPUTS], an 8-element static array. Nothing stops a host sending more ZcashTransparentInput messages after the declared count is consumed. The ack loop stops asking, but the handler still accepts them, and current_transparent_input keeps incrementing -- so index 8, 9, 10 and beyond each wrote a fully host-controlled ZcashTransparentInputState past the end of the array: amount, a 32-byte prevout txid, the script_pubkey and the whole address_n array, straight into whatever follows it in static memory. Reject an index outside the declared count or the array bound, and refuse once the expected inputs have all arrived.
The review chain ran VERIFIED, then `else if (schema_verified)`, then OPAQUE, and only rejected SOL_TX_REVIEW_MALFORMED in the final else. So the schema arm shadowed the rejection: an attested KKSOLSC1 schema matching one partially-parsed instruction was enough to reach the schema review screens and then the signature, on a message the parser had already declared malformed. A schema describes a single instruction; it says nothing about the rest of a transaction that failed to parse. Reject malformed up front, as its own guard rather than by restoring it to the end of the chain -- branch ordering is what hid it the first time. The final else stays as a backstop for any future review state.
…igning fsm_msgEthereumTxMetadata already refused to run during signing, with a comment explaining exactly why: clearing the binding mid-flight lets a host approve a benign decode, suppressing the blind-sign gate, then stream attacker-chosen calldata for the remaining chunks. Two other paths reached the same clear with no such guard. fsm_msgLoadClearsignSigner ends in signed_metadata_store_signer() -> signed_metadata_clear(), which drops stored_metadata along with relied_on_metadata. It now carries the same prologue as its sibling. fsm_msgClearSession was worse: an unauthenticated MSG_IN needing no button press, which called session_clear() -> signed_metadata_clear_signers() -> signed_metadata_clear() while Ethereum signing continued across it. signed_metadata_enforce() then saw relied=false and passed. It aborted only Zcash; fsm_msgInitialize and fsm_msgCancel abort every engine, and so does this now. The bitcoin-only build already provides an ethereum_signing_abort() no-op for exactly these always-on handlers.
…dition The five BIP-341 SHA-256 contexts were initialised inside the non-overwintered `else`, while every update and finalise site for them is gated only on `coin->has_taproot && coin->taproot`. `overwintered` arrives from the host on SignTx, so setting it on a taproot coin skipped the initialisation and left the use sites live: the sighash was then built over whatever those static contexts held -- uninitialised on the first signature after boot, and carried over from the previous transaction afterwards. Initialise them on the taproot condition alone, outside the overwintered branch. Whether a non-Zcash coin should accept overwintered at all is a separate question and deliberately not changed here.
…, size the bech32 buffers Three things, one of which is a self-inflicted regression the new device test caught within one CI run. 1. RESTORES the Zcash shielded two-screen output confirm. b21f52c was committed from a working tree that had silently rolled back to the pre-fix content, so it reverted the fix from 56807a7 while adding the index bound. Nothing in the diff summary showed it. The device test added for exactly this defect failed on the next run with "expected 2 ConfirmOutput screens, got 1" -- the same message it produces against the unfixed release build. An audit of every fix on this branch found this and the output bound below were the only two casualties; the other eleven survived intact. 2. Bounds the transparent OUTPUT index, the sibling of the input overflow. Fixing fsm_msgZcashTransparentInput did not cover fsm_msgZcashTransparentOutput: a different array with its own free-running counter. After the declared outputs are stored the dispatch moves to transparent inputs without incrementing current_transparent_input, which leaves the output handler re-armed -- so a host that ignores the ack and keeps sending outputs walks current_transparent_output past n_transparent_outputs, writing a host-controlled amount and a 128-byte script_pubkey past transparent_outputs[8], onto transparent_inputs[0] and then outside the struct. 3. Sizes every Tendermint-family bech32_decode buffer to its documented contract. segwit_addr.h requires hrp >= BECH32_MAX_HRP_LEN + 1 (84) and data >= strlen(input) - 8. Every caller in signtx_tendermint.c, osmosis.c, thorchain.c, mayachain.c, tendermint.c and binance.c used char hrp[45] and uint8_t decoded[38] against address fields whose proto max_size is 53 -- so a long address overflowed both. bech32_decode fills these buffers BEFORE validating the checksum, so the usual `if (!bech32_decode(...)) return false;` guard never prevented it: a rejected address had already written past the end.
draw_string() renders characters until one no longer fits the canvas and then stops. Only BODY_ROWS rows of body text are on screen -- row 4 starts at y=66 on a 64px display -- so any longer body was drawn in part, with no ellipsis and nothing to tell the user that something had been cut. The worst instance is the ERC-20 approve screen, which for any realistic amount or a ticker past five characters drew 39 of the spender's 42 hex characters and dropped the trailing '?'. An attacker who grinds an address matching a known-good contract in its visible prefix gets a screen the user cannot distinguish from the real one. It is one of ~30 sites: Solana stake authorities, TRON recipients, THORChain asset chains, the "NOT verified by KeepKey" warning and the passphrase echo all lose their tails the same way. Fix it once, where every confirm passes through. confirm_helper() now splits an over-long body into screen-sized pages, each with its own ButtonRequest and its own press, and titles them "1/2", "2/2". Bodies that already fit take exactly the path they did before -- same single screen, same title, no extra press -- so only the flows that were already broken change shape. Custom layouts are left alone: they place and size their own body, and layout_zcash_address_text_notification deliberately renders 106 characters as 38+38+30. calc_str_page() gets an early exit. Line count cannot fall as a prefix grows, so the first prefix that does not fit settles the page; without it the scan ran the full body length for every page and a 350-character confirm paused for roughly a second on device. Verified: three board unit tests drive confirm_body_split() over the real ethereum.c bodies -- pages reproduce the body exactly, no page exceeds BODY_ROWS, and fitting bodies stay on one page. Both mutants fail them: forcing a single page reproduces the original truncation, and allowing four rows trips the overflow assertion. The early exit was checked against the old full scan over 19,200 random string/width/row combinations.
… wrong b024f8b made confirm() page a body too long for BODY_ROWS instead of silently dropping its tail. Two memo vectors cross that line, so the swap tests waited on presses nobody queued and hung until CI killed the job: unit-tests at its 10-minute timeout, python-integration-tests at its 30 — both reported "cancelled", which reads like flake rather than a wrong expectation. python-integration-tests runs firmware-unit before the pyk suite, so the pyk tests never ran at all. Measured against the real font tables rather than guessed: Confirm swap asset USDT-0xdac17f95...\n on chain ETH 4 rows -> 2 pages Confirm to 0x<238 d's> 8 rows -> 3 pages so MemoSwapFullFormShowsAffiliate is 5 presses (2+1+1+1) and MemoExactBufferCapacityKeepsLastChar is 6 (1+3+1+1), in both the thorchain and mayachain suites. Nothing else in the suite pages: the 42-character destination addresses land on exactly 3 rows and still fit. That the USDT screen paged is the bug being fixed showing itself — the tail of that contract address was never reaching the display. kkconfirm_preload() now queues one trailing rejection as a sentinel. An unbudgeted screen consumes it, is rejected, and the call under test returns false immediately, so a wrong count fails in milliseconds with a negative drain instead of hanging; kkconfirm_drain() discounts the sentinel so existing "== 0" assertions keep their meaning. Verified by mutation: with preload(4) against 5 screens the test fails in 5 ms on drain() == -2. 405/405 firmware-unit tests pass.
…se/715-audit-06-bitcoin-security fix(release): unbreak the tag build, and make the report show what CI actually ran
1 participant
What changed
python-keepkeyrelease-client pin only after all firmware features are presentWhy
This is the final Bitcoin/security and integration gate. Deferring the client pointer here prevents earlier PRs from failing against future-feature tests while ensuring the complete release is exercised end-to-end.
Validation
actionlint, gitleaks HEAD scan, Pallas API-boundary gate, andgit diff --checkpassedStack: 6 of 6. Review only the diff from
release/715-audit-05-chains.