fix: render multiline signed messages as text - #319
Closed
BitHighlander wants to merge 62 commits into
Closed
Conversation
Full 7.15.0 feature set as one reviewable unit: - EVM clear-signing: signed-metadata v1 + v2 static schema, runtime LoadClearsignSigner (warning-gated, no built-in keys in phase 1), persistent identities + trust icons (STORAGE_VERSION 17->18) - EVM signing hardening: RLP zero-value canonicalisation (EIP-1559), EIP-712 cancel propagation, token chain_id widening - New chains: Hive SLIP-0048 (SignTx / SignOperations / SignMessage / account ops), Zcash Orchard shielded (PCZT signing, FVK export, unified addresses, signing progress UX), Ripple memos, THORChain any-denom - Per-chain clear/blind-sign: TRON, Solana v0, TON, Maya/THOR affiliate - Robustness: BIP-85, BIP-39 recovery validation, fault-injection hardening, SRAM frame arena + 16KiB reserve gate + CI budget - Build: bitcoin-only / zcash-privacy variants + seed-lock, 3-variant CI matrix, emulator dylib (Windows cross-compile, confirm gating) Pins: device-protocol 4eb7d5e / python-keepkey c475482 (tips of keepkey up/release-protocol and reconcile/upstream-sync; pin-swap to master follows device-protocol#112 / python-keepkey#197).
…eld and cross-domain attacks Ten clear-sign findings from security review, fixed while keeping clear-signing functional (display everything; never degrade to blind-sign for legit memos). - signed_metadata: v2 schema cannot bind msg->value, so refuse to clear-sign a payable tx under v2 (the native-value screen is suppressed on a v2 match). - thortx: bind THOR/MAYA router to chain_id==1; display the real msg->value for native deposits (router ignores the ABI amount) and reject amount/value disagreement; read the ABI memo length instead of a fixed 64 bytes and bound the padded calldata exactly; lower min-chunk to 164/196 so short memos still clear-sign. - thorchain: display DEX-aggregator (agg addr, final token, min out) and ADD affiliate/fee fields; page the complete raw memo when a '|' outbound suffix is present (it can carry ':' the split would scatter). - hive: pin tx chain_id to mainnet; reject non-printable messages so a binary transaction preimage (chain_id||tx) can never be signed as a message on any fork chain id (closes the cross-domain signature oracle). - solana: page the full memo (text/hex) instead of a byte-count summary; show the real mint next to the host-supplied, unauthenticated token symbol. Adds unit tests for the THORChain aggregator/ADD memo display and the Hive printable-message guard.
… token symbol
Second-round review follow-ups:
- Native THOR/MAYA deposit and send handlers signed after a single unpaged
confirm("%s", memo) when the structured parser returned false, so parser-level
rejects never actually rejected and long memos (THOR up to 250 bytes)
truncated. Every native memo site — and the EVM router path — now pages the
COMPLETE raw memo (thorchain_confirm_full_memo, now shared/public) after the
best-effort structured summary; a reject on any page aborts signing.
- This raw-memo disclosure also backstops mayachain.c, whose structured parser
had not been hardened (ignored aggregator/final-token/min-output and ADD
affiliate/fee) — those fields are now shown in full via the paged raw memo.
- Solana token symbols are host-supplied and only length-capped; a symbol with
newlines could push the mint/recipient off-screen. Restrict symbols to
printable ASCII (solana_symbol_is_safe) and show the authenticated mint on its
own screen before the amount; an unsafe symbol falls back to a raw token count.
Adds unit tests for the raw-memo pager (short/long/reject/binary).
…nt!=value CI integration tests surfaced two problems in the earlier native-deposit hardening: - Native ETH is expressed either as address(0) or the 0xEeee..Ee sentinel; the is_native check only matched address(0), so a native deposit whose asset used the sentinel was mis-classified as a token and rejected for carrying value. Match both, comparing exactly 20 bytes. - Requiring the ABI amount to equal msg.value was too strict: the amount word is a router-ignored hint that legitimately differs from the forwarded value. Displaying msg.value (the amount actually sent) already closes the display-0.01-send-100 gap, so the equality check is dropped. Bumps python-keepkey to invert the Hive non-printable-message test to expect the new rejection. Full python-keepkey integration suite passes locally (546/0).
… native-memo abort, empty-memo UB, native-ETH Third-round security review follow-ups: - Solana stake/vote withdrawals and stake split displayed only the amount, so a host could redirect the withdrawn SOL unseen. Show the recipient (and the vote account for delegate). - Unchecked SPL Transfer/Approve carry no signed mint — the token being moved or delegated cannot be proven, so they now require AdvancedMode (blind-sign); only the *Checked variants (mint signed + displayed) clear-sign. - Native THOR/MAYA handlers ignored the structured parser's bool return, which conflates "unrecognized" with "user rejected", so a rejected screen could be followed by raw-memo pages and then signing. Drop the structured pass; the raw memo pager is the sole, authoritative gate, so a reject aborts. - The raw-memo pager read an uninitialized buffer for an empty memo (len==0 fell into the hex branch). Return a "(empty)" screen; add a unit test. - Accept only address(0) as native ETH for the pinned THOR/MAYA routers: the 0xEeee sentinel is treated as a token on-chain and would clear-sign a tx that reverts. Bumps python-keepkey (fixture + Solana AdvancedMode tests). Full integration suite passes locally (546/0).
…s, opaque gating Fourth-round Solana review (audited against Trezor and Ledger, which both use an all-or-nothing model and refuse unchecked SPL transfers): Correctness: - solana_signTx signed msg->raw_tx from offset 0, but solana_inspectTx parsed after stripping a leading 0x00 signature-count byte — so an unsigned-tx-format payload was signed as 0x00||message and would not verify on-chain. Normalize one message slice (solana_message_slice) for both parse and sign, and reject any signature that fails ed25519 self-verification. - StakeAuthorize read a le32 authorize-type at offset 36 but only required 36 bytes — a 4-byte over-read; require >= 40. - thorchain full-memo pager: "ascii = len > 0" is always true after the empty guard (cppcheck); use true. Display completeness (fields were captured but hidden): - MintTo shows mint + recipient; Burn shows mint + source; StakeMerge shows source + destination; Stake/VoteAuthorize show the role (staker/withdrawer, voter/withdrawer) + new authority; InitializeNonce shows the authority; ATA create shows owner + mint; Freeze/Thaw show the account. Opaque gating (cannot be fully/safely disclosed → AdvancedMode blind-sign): - CreateAccount (assigns owner program + space, not parsed) and SetAuthority (account-takeover vector; the clear-to-None case isn't distinguished). Bumps python-keepkey with tests for the new gates. Full integration suite passes locally (549/0); cppcheck clean.
…clear-sign anchor) The EVM clear-sign trust model — a host attestation verified against a signer the user loaded via LoadClearsignSigner — is chain-agnostic. Reuse it so a Solana token's symbol can be authenticated instead of trusted from the host. - signed_metadata_verify_attestation(): generic "verify an ECDSA signature over sha256(data) against a loaded/pinned clear-sign signer" primitive. - SolanaTokenInfo gains optional signature + signer_key_id (device-protocol bump). A TransferChecked's host symbol is trusted only when its signature verifies over a domain-separated (mint, decimals, symbol) digest; a present- but-invalid signature rejects the symbol outright, and no signature keeps today's behavior (symbol shown beside the always-authenticated mint). A verified symbol labels its mint screen "Verified token mint". Purely additive and dormant: no current host sets a signature, so behavior is unchanged until the host/SDK signing side (a follow-up) lands — mirroring how EVM clear-sign shipped firmware-first. Also updates Solana.ParseSPLTokenTransfer to expect OPAQUE (unchecked SPL transfers now require AdvancedMode). Adds an attestation unit test. Full gtest suite (277) + integration (549) green locally; cppcheck clean.
The Solana signing self-verification pulls in ed25519_sign_open (the ed25519 verification path), the only such use in the firmware. That overflowed the ROM-tight zcash-privacy variant by ~3.3KB. The real correctness guarantee is solana_message_slice() making parse and sign use the identical bytes; the self-verify is defense-in-depth, so compile it out only on zcash-privacy. Verified: zcash-privacy ARM firmware.keepkey.elf now links.
…ow-ups) - Name the attesting signer instead of a generic "Verified": a verified symbol now shows "Token <sym> signed by <alias>" (all signers are user-loaded, not built-in KeepKey keys), matching the EVM clear-sign identity model. - Require the attested decimals to equal the signed instruction's decimals (pi->extra_u8) before trusting the symbol, so an attested (mint,decimals, symbol) tuple that disagrees with the transaction cannot earn "verified". - Reject signer_key_id >= METADATA_MAX_KEYS before narrowing the uint32 proto field to the uint8 keyring slot, so 256 can't alias slot 0. - Move solana_token_info_trusted to a public function and add a unit test that builds the real production preimage (tag || mint || decimals || symbol), signs it, and exercises acceptance plus every rejection branch (decimals mismatch, bad signature, out-of-range key id, no signature). gtests 278, integration 549, cppcheck clean; zcash-privacy ARM links.
…lidator; signer fingerprint Ten Solana completeness/correctness review findings: - Show WHICH account is acted on (parsed but previously hidden), via a shared solana_confirm_account helper (keeps the added screens ROM-cheap): System Assign (account + owner), Stake/Vote Authorize (the account being rekeyed), Stake Delegate (stake + vote), Stake Deactivate, Vote UpdateCommission, and Token CloseAccount (source + balance destination — closing sweeps the whole lamport balance the device can't see). - Token-2022 TransferChecked can invoke an undisclosed transfer hook / fee, so force it opaque (AdvancedMode); only legacy SPL TransferChecked clear-signs. - Vote UpdateValidatorIdentity has no data payload — it was displaying 32 attacker-supplied trailing bytes. Require the canonical 4-byte encoding and read the new validator from account index 1. - The verified-token screen now shows alias + signer fingerprint (aliases are not unique) and drops the "unknown" fallback from the verified path (signed_metadata_signer_fingerprint). Adds a Token-2022-opaque gtest. gtests 279, integration 549, cppcheck clean; zcash-privacy ARM links.
…s, + tests - Show the priority fee as an actual SOL cost: when a tx sets a compute-unit price, display the fee payer and the MAXIMUM priority fee (ceil(price*limit/1e6) lamports, overflow-safe, using the 1.4M-CU cap when no explicit limit is set so the figure is never understated). Raw CU price/limit with no units could otherwise hide a large SOL loss (fees are charged even on failed execution). - Show the acted-on account (previously parsed but hidden) for nonce withdraw/authorize/advance/initialize, Allocate, stake/vote withdraw, stake split, token Revoke and SyncNative; Freeze/Thaw now also show the mint. - Add parser regression tests for the validator-identity fix: canonical UpdateValidatorIdentity reads the new validator from account index 1, and a trailing-byte (non-canonical) encoding is rejected as opaque. gtests 281, integration 549, cppcheck clean; zcash-privacy ARM links.
…path Priority-fee ceiling arithmetic ((price*limit + 999999)/1e6) could overflow the +999999 rounding term even when price*limit itself fit, wrapping the displayed fee to zero while the real cost was up to ~1.8e10 SOL. Replace with overflow-safe quotient/remainder ceil in a public, testable solana_priority_fee_lamports(); it returns false (caller refuses to sign) rather than saturating when the true value exceeds UINT64_MAX. Route both fsm_msgSolanaSignTx and fsm_msgSolanaSignMessage through one solana_confirm_verified_tx() (per-instruction disclosure + priority-fee screen) so a transaction-shaped SignMessage payload can no longer dodge the fee screen and the two entry points' security screens cannot drift. gtest PriorityFeeOverflowSafe covers UINT64_MAX/1 -> 18446744073710, the non-overflow cases, and UINT64_MAX/UINT64_MAX -> reject.
…e sign-message/ops + solana mint & attested-symbol + EVM deposit flows, fix frame picker
Bump deps/python-keepkey to 71d6086:
- mayachain native signtx tests un-skipped (stale thorchain-copied vectors
replaced with host-side sign-doc digest verification against the known
device pubkey) — native MAYA raw-memo paging now runs and captures OLED.
- new solana tests: TransferChecked clear-sign (dedicated 'Token mint'
screen, AdvancedMode OFF) and signed-token-def attestation ('Token "USDC"
by <alias> <fp>' via LoadClearsignSigner); messages_solana_pb2 regenerated
with the optional SolanaTokenInfo signature/signer_key_id fields
(device-protocol 47e19d8).
- report SECTIONS: Hive G6-G28 (sign-message printable-whitelist oracle fix
+ sign-ops + fences), Solana S8/S12 captions corrected to force-opaque +
S13-S24, Maya M4-M6, THOR H2 full raw-memo sequence + H4, EVM E20/E21
deposit happy paths captured.
- frame picker: blank/lock frames never rendered; cross-test duplicate
census ranks test-specific frames above shared chrome (fixes the leaked
IMPORT-RECOVERY/blank frames); outcome frames after the lead frame kept
for gate tests.
Validated locally (scripts/emulator docker compose): 554 passed / 0 failed /
16 skipped; screenshot pass 176 tests, 824 PNGs; report 283 tests, 277
passed, 0 pending.
release(7.15.0): consolidated release delta — full 7.15 feature set, one reviewable unit
…st mainnet Re-port of #309 onto the consolidated line. thor_isThorchainTx pinned the router to the Ethereum-mainnet address, so a THORChain depositWithExpiry on Avalanche never matched and fell into the AdvancedMode blind-sign gate (bare ActionCancelled). Scope the pin to (chain_id, address): thor_router_for_chain returns the router for this tx's chain — mainnet d37bbe.., Avalanche C-Chain 00dc61.. (verified live against /inbound_addresses) — or NULL (unpinned chain -> blind-sign gate). Differences from #309, deliberate: - A tx with NO chain_id gets no router at all (fail-closed), rather than inheriting mainnet from ethereum.c's hashing default — an identity pin must never come from a default the host merely omitted. - #309's memo-length fix is already on this line in a stronger form (exact padded-end bound, whole-calldata-in-chunk); its tests are ported against those semantics. - Native deposits now format with the CHAIN's native ticker via ethereumFormatAmount(NULL token) — the 0xEE pseudo-token is pinned to " ETH" and mislabeled every other chain's native amount ('0.5 ETH' for 0.5 AVAX). Adds the missing 43114 -> AVAX ticker case. BSC (56) and Base (8453) routers remain unpinned until verified against a live node (the shipped Pioneer catalog's AVAX entry is already stale, and those pairs route via Relay today). Tests: 8 firmware unit (chain-scoped match/reject both directions, unpinned chain/address, missing chain_id, AVAX full-confirm 67-byte memo, over-declared memo length rejected) — 290/290; pyk +2 (AVAX deposit ECDSA-recovered over chainId 43114 + unpinned-chain gate) — 556 passed/0 failed; OLED shows 'Confirm sending 0.5 AVAX'; report E23/E24. ARM zcash-privacy elf links (ROM budget holds). deps/python-keepkey -> d39ab36. Supersedes and closes #309.
… hardening Round-12 review remediation (5 majors + one-liners + regression tests). Memo disclosure (majors 1-3): - ripple: route the memo through thorchain_confirm_full_memo — the single unpaged confirm rendered only 3 OLED lines and honored embedded newlines, so up to ~180 signed bytes of Memos-field routing data could be invisible. - tron: same pager replaces the <=114-char single screen (3 lines only guarantee ~84 wide-glyph chars, clipping a signed affiliate/destination tail); non-printable memos now page as full hex. - thorchain/mayachain WITHDRAW: display the 4th field (asymmetric single- sided payout asset selector — it directs money) and cap nfields>4, matching the SWAP(>9)/ADD(>5) caps. Screens for -:BTC.BTC:BPS:ASSET were otherwise identical to the symmetric form. CI (major 4): the emulator CMake default is KK_ZCASH_PRIVACY=ON, so the empty-flag full leg built identical to the zcash-privacy leg and the SHIPPED device config (multi-coin, privacy OFF) was never unit-tested. Pin the full leg to -DKK_ZCASH_PRIVACY=OFF; publish-emulator now ships the zcash-privacy image (same bytes the old full leg produced). Submodule pins (major 5): deps/python-keepkey -> 8586652 (declares device-protocol from keepkey/up-release-protocol, now fast-forwarded to the 47e19d8 pin). Fork device-protocol and keepkey/up-release-protocol both hold the pin, so 'submodule update --remote' can no longer rewind below the proto fields / hive tests the release depends on. Pre-tag one-liners: storage.c V18 buffer comment arithmetic (3479/3480); BITCOIN_ONLY GetFeatures emits the bitcoin-only-locked sentinel; solana TransferChecked requires the canonical 10-byte data + >=4 accounts else opaque (a short encoding showed a zeroed 1111.. destination); zcash pins enc_noncompact.size==16; signed_metadata applies the printable-ASCII allowlist to method/arg names too; release.yml runs check_sram_budget.py + requires green CI on the tagged SHA before producing artifacts; release.sh passes extra args through to cmake for per-variant reproducible builds. Regression tests: signed_metadata v2 nonzero-native-value reject; solana TransferChecked canonical/short-data/short-accounts + StakeAuthorize 36-vs-40-byte; zcash orchard transparent-sig-digest S.2-vs-T.1 (the shield fix site, previously uncalled); thorchain WITHDRAW asymmetric asset + field-cap. Firmware unit + pyk suites validated locally.
fix(7.15): per-chain THORChain router + round-12 review remediation
The trickle eased half its span in 1.2s then crawled asymptotically, so during a 40-50s host proof window the bar looked frozen; the full-track sweep marker (33c2f37) compensated by gliding an 8px notch across the whole bar, which appears instantly at the track start and detaches from the fill, leaving a visible gap. Replace both: - Ramp linearly through 90% of the milestone span over an expected 45s window, then hold — the last 10% is only crossed by the next real milestone, so the bar never claims work that hasn't happened. - Replace the detached sweep with a glint on the fill's leading segment that breathes dim<->bright each 1.6s. Activity always shows exactly at the progress front: no gap, and it cannot run out of travel as the unfilled span shrinks near 100%.
Two windows left a dead screen (the dismissed dialog) for 10-20s: - after approving an Orchard output, RedPallas signing runs before the first progress draw; - after the initial Shield/Shielded confirm, Orchard ZIP-32 key derivation runs before the first draw. Both are pure device-side Pallas math with no display update. Draw the 'Signing Zcash' bar immediately on approval, before the heavy work. The transparent-output path already drew immediately (no change).
…lint fix(zcash): smooth 45s progress ramp + fill-front glint
The device clear-sign table held only vote/comment/custom_json, but the
client and vault already serialize nine more. Those nine were rejected
on-device with "unsupported operation type" — shipped host code with no
firmware to sign it. Separately, dApp swaps on the Hive internal market
failed at the extension with "Operation not in the KeepKey clear-sign
table (got limit_order_create)".
Adds to hive_parseOperations + fsm_msgHiveSignOperations:
3 transfer_to_vesting 19 comment_options
4 withdraw_vesting 32 transfer_to_savings
5 limit_order_create 33 transfer_from_savings
6 limit_order_cancel 39 claim_reward_balance
8 convert 40 delegate_vesting_shares
43 account_update2
Ops 2/9/10 remain permanently excluded. Anything outside the table is
still refused outright — no blind-sign fallback.
New parser primitives:
cur_asset pins each asset's symbol to a per-op whitelist AND to that
symbol's protocol-fixed precision. Both are load-bearing for
display integrity: a swapped symbol hides a ~2000x value
difference behind an identical-looking number, and a wrong
precision moves the decimal point relative to what the chain
applies. Negative int64 amounts are rejected — they would
render as enormous positive values.
cur_bool rejects any byte but 0/1 rather than coercing it.
cur_u16/32 bounds-checked fixed-width readers.
Enforced on-device, not merely host-side:
- comment_options is accepted ONLY immediately after a comment op with
the same author and permlink. Detached, it could redirect payout of a
post published earlier that the user is not reviewing on screen.
- account_update2 hard-rejects any authority field — the op-9/10
device-derived-keys invariant applied field-level.
- beneficiaries must be strictly ascending, unique, and sum <= 100%.
- zero amounts are refused where they are meaningless and allowed where
they carry meaning (withdraw_vesting 0 stops a power-down,
delegate_vesting_shares 0 removes a delegation).
Display: every screen is capped at three body rows. layout.c places rows
at y=24/38/52 and draw_char_with_shift silently drops any glyph past
y+height > 64, so a fourth row would be signed but never shown.
claim_reward_balance therefore spans two screens rather than losing its
VESTS line, and account/amount fields never share a row with a label
where a 16-character name could push the destination off-screen.
Flash is the binding constraint on this part (0xA0000 total, ~7.4KB free
before this change), so rejection reasons are grouped by cause instead of
one bespoke sentence per failure site. They are diagnostics, not security
surface — the protection is that the device refuses, and the host knows
what it sent. The three carrying distinct security meaning stay separate.
Net cost 3,664 bytes; 3,784 bytes remain free.
Tests: 23 Hive cases (was 4), including per-byte truncation sweeps, asset
symbol/precision/sign rejection, the comment_options binding invariant,
authority-change rejection, tier assignment, and mixed-tier refusal.
322/322 firmware-unit pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Picks up feat/hive-clearsign-ops-phase3 in keepkey/python-keepkey: device tests for the 11 new clear-sign ops, plus two fixes to existing tests that this firmware change invalidated — three assertions matching rejection strings that are now grouped by cause, and an unknown-op case that used op type 3 (now transfer_to_vesting, so it no longer reached that path). 38/38 hive device tests pass against an emulator built from this branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cppcheck funcArgNamesDifferent x3 — the only red check on this PR. Zero-warning policy, so CI fails on style findings. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… no longer has Adding the phase-3 Hive op table pushed the zcash-privacy variant 2136 bytes past its ROM budget: ld: section `.data' will not fit in region `rom' ld: region `rom' overflowed by 2136 bytes The three sscanf calls here were the firmware's only callers of newlib's scanf engine, which links in ~6KB to parse two hex digits and a decimal chain id. Both jobs already have cheaper answers in this file: encAddress() two lines up parses hex with strncpy + strtol, and strtoul covers the chain id on device and emulator alike, so the EMULATOR ifdef goes with it. text drops 653884 -> 647884, leaving the zcash-privacy build ~3.9KB of headroom. full and bitcoin-only are unaffected in behaviour. An unparseable chain id now yields 0 instead of leaving chainInt uninitialised, which is the only behavioural change.
…-sscanf Removing the sscanf calls let cppcheck see through to the real scope of these two: `ctr` is only used inside its own for-loop, and `assetToken` is only used and read inside a single `if` block below. CI's static-analysis job runs with a zero-warning policy, so this was blocking the branch.
…float engine Every Osmosis confirm screen (send, delegate, undelegate, LP add/remove, redelegate, swap, IBC transfer) rendered amounts with atof() + "%.6f". A float carries ~7 significant decimal digits, so any amount past that on the screen the user is asked to approve got silently rounded — e.g. 123456789123456 uosmo showed as "123456792.000000 OSMO" instead of "123456789.123456 OSMO". osmosis_formatAmount() does the same base-unit scaling in integer math via bn_format_uint64 (bignum.h), the formatter the Hive and Ethereum confirm screens already use, and is exact at any magnitude. It also drops the last callers of atof/pow/float-printf in this file, which pulled newlib's floating-point engine into a ROM budget the zcash-privacy build no longer has: text drops 647884 -> 639796 (zcash-privacy) and 645576 -> 633956 (full), an ~8KB win on both. Moved into osmosis.c/.h (mirroring hive.c) so it's independently unit tested rather than only reachable through fsm_msgOsmosisMsgAck.
pyk 3a9e1b7. The first Osmosis device tests exposed that osmosis_sign_tx's MsgSend branch had never run: it whitelisted the Cosmos denom (uatom), dropped the denom field, and assigned an int to a string proto field.
…rdening-rc17 RC17 security hardening, exhaustive automated evidence, and completed physical smoke gate.
fix(release): support RC tags and selective submodules
13 tasks
Owner
Author
|
Superseded by #320. The exact multiline signed-message patch and its regression coverage are included in the RC18 audit candidate, which now has a fully green hosted matrix. Closing this duplicate review surface so RC18 remains a single firmware PR into |
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
1 participant
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.
What changed
0x0a) line breaks as text in the shared byte-confirmation pager.Root cause
confirm_bytes_is_ascii()required every byte to be in0x20..0x7e. A normal multiline EIP-191 message contains LF bytes, so the entire otherwise-readable message was rendered as hexadecimal on the physical device. This made user consent effectively blind even though the signed bytes were valid UTF-8 text.The pager also did not count explicit LF bytes as rendered row advances, which could undercount pages if LF were simply admitted by the classifier.
Impact
Multiline EIP-191 and other text message payloads now render as readable, paginated text on-device. Binary/control-bearing payloads continue to use exact hex disclosure.
Validation
AppConfirm.*: 2/2 passed.clang-format --dry-run --Werrorandgit diff --checkpassed.