[rehearsal 1b] Per-chain clear/blind-sign: TRON, Solana v0, TON, Mayachain, THORChain affiliate#297
Open
BitHighlander wants to merge 14 commits into
Open
[rehearsal 1b] Per-chain clear/blind-sign: TRON, Solana v0, TON, Mayachain, THORChain affiliate#297BitHighlander wants to merge 14 commits into
BitHighlander wants to merge 14 commits into
Conversation
fsm_msgTonSignTx displayed to_address and amount from deprecated proto fields that are NOT included in the raw_tx bytes being signed. A malicious host could show one transfer on screen while getting a different transaction signed — identical CVE pattern to TronSignTx. Replace with a single blind-sign prompt showing only the raw_tx byte count. (cherry picked from commit cf288b9)
fsm_msgTronSignTx displayed to_address and amount from deprecated proto fields that are NOT included in the raw_data bytes being signed. A malicious host could show one transfer on screen while getting a different transaction signed. Replace with a single blind-sign prompt that shows only the raw_data byte count, which is the actual data committed to by the signature. (cherry picked from commit 42f19cb)
TIP-712 signing sends only pre-computed hashes — the device cannot reconstruct or verify the original typed-data struct. Typed-data signatures can authorise token approvals and permits. Add an explicit acknowledgement screen before showing the raw hashes so users understand they are signing unverifiable data. (cherry picked from commit 2fbd1ea)
For SOL_INSTR_TOKEN_TRANSFER_CHECKED the instruction data encodes the authoritative decimal count in byte 9 (parsed into pi->extra_u8). The host-supplied SolanaTokenInfo.decimals is metadata only and could be forged to misrepresent the displayed amount. Split the fallthrough case so TRANSFER_CHECKED always uses pi->extra_u8 for decimal scaling while still displaying the host-supplied symbol as a label. (cherry picked from commit 2690731)
…_data The device signs sha256(raw_data) but displayed nothing about it — every TRON transaction was a 'TRON Blind Sign / Sign N-byte transaction?' screen. Pioneer-powered swaps from TRC-20 (USDT) were fully blind. Parse raw_data on-device (a minimal fail-closed protobuf reader over the exact signed bytes) and clear-sign the two payloads the Vault builds: - TransferContract: 'Send <amt> TRX to T...?' + memo (THORChain swap memos ride in raw_data.data) + fee_limit - TriggerSmartContract carrying transfer(address,uint256): token contract, recipient, amount in base units, fee_limit, memo Owner address must match the derived key (parsed from signed bytes). Anything not fully understood — extra contracts, unknown fields, Permission_id, TRC-10 value, non-transfer selectors, dirty ABI words — stays on the blind path, now gated behind AdvancedMode like Solana's opaque transactions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit fa31202)
…ting pb_read_varint accepted a 10-byte varint whose final byte's payload had bits set above bit 0 (shift=63 + 7 bits > 64), silently discarding the overflow via the left-shift. A malformed key, length, amount, or fee_limit varint using more precision than 64 bits allows could parse as if it were well-formed instead of falling to TRON_TX_UNVERIFIED. Reject outright when the 10th byte's payload exceeds 1. Added tests for overlong key, length-delimited length, amount, and fee_limit varints. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 991db99)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 8af7248)
…enom handling Swap-memo clear-signing (thorchain.c + mayachain.c parseConfirmMemo, deliberate near-copies kept textually parallel): - Replace the strtok tokenizer with a manual ':' split that PRESERVES empty fields. strtok collapses consecutive delimiters, so an empty field (e.g. the limit in "=:ETH.ETH:0xdest::kk:75") silently shifted later fields left and the affiliate would have been displayed as the limit on a security screen. - NEW 4th swap confirm screen "Affiliate fee <bps> bps to <affiliate>" whenever the affiliate field is non-empty (fee shows "0" when absent). Previously memo fields 5-6 (affiliate + fee bps) were silently dropped, so users never saw affiliate fee skimming. - SWAP/ADD/WITHDRAW intent detection, confirm titles/strings, "self"/"none" defaults, ADD's optional pool screen and WITHDRAW's required basis-points field keep their existing behavior. Memos whose second field has no chain.asset '.' pair now consistently fall back to raw-memo display. MAYA hardening to THOR parity: - Add mayachain_isValidDenom (copy of thorchain_isValidDenom) and validate the MsgSend denom (default "cacao" when absent/empty) in fsm_msgMayachainMsgAck BEFORE any display, failing with Failure_SyntaxError "Invalid denom". Previously arbitrary denom bytes flowed unvalidated into the UI and the signing JSON (snprintf'd into the amount object in mayachain.c). - Replace the raw sprintf into denom_str[71] with a bounded snprintf, and show the validated/defaulted denom on the final sign screen. - MsgDeposit display (both chains): widen asset_str 21 -> 64 and amount_str 32 -> 96. Long-form THOR assets (~50 chars, e.g. ETH.USDT-0XDAC17...) were truncated at 19 chars, and bn_format zeroes its output on overflow so the old 32-byte amount buffer would have rendered long-asset deposits as an empty amount. Tests (make xunit green: 200 firmware + 2 board + 4 crypto): - New parseConfirmMemo suites for both chains driven end-to-end through the real confirm() loop: tests preload ButtonAck + DebugLinkDecision tiny-message frames on the emulator UDP port and assert the exact number of confirm screens consumed, proving the affiliate screen exists, empty fields don't shift, and legacy ADD/WITHDRAW screen counts are unchanged. - Re-enable unittests/firmware/mayachain.cpp (stale 2-arg mayachain_signTxUpdateMsgSend call since 50164a2); pass "cacao" to match the historical JSON. The recorded signature vector turned out to be cryptographically invalid for the fixture's key/JSON (the file was never in the unit build, see 28c74a0, so it was never validated); replaced with a vector recomputed independently via python-ecdsa (RFC6979/secp256k1, low-s) that the firmware output matches exactly. - Add mayachain_isValidDenom validation vectors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit ca7c569)
… honest wording for unspecified affiliate fee thorchain_parseConfirmMemo/mayachain_parseConfirmMemo received size as a raw byte count from the BTC OP_RETURN caller (transaction.c passes op_return_data.size, no NUL), but copied the memo with strlcpy(dst, src, size), which writes only size-1 bytes — silently dropping the memo's final character. An affiliate fee of '75' bps displayed as '7'; a one-character affiliate vanished entirely. Copy the bytes exactly (bounded memcpy into the zeroed buffer) instead. Also: an empty fee field with an affiliate present displayed 'Affiliate fee 0 bps' — but THORName/MAYAName registration lets the network apply the affiliate's registered default bps when the memo field is empty, so '0' asserted a fact the device cannot know. Say 'unspecified' instead. Regression tests pass raw non-NUL-terminated bytes and assert the exact screen count (the dropped 1-char affiliate showed 3 screens, not 4). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 6e6cbcb)
…-by-one at full buffer capacity mayachain_signTxUpdateMsgSend still wrote denom into the signed JSON with an unescaped %s — the FSM caller validates first, but the exported signer helper itself had no defense, unlike thorchain's hardened path. Add the same default+validate+escaped-write sequence directly in the signer function so it stays safe if called directly or reused later. Also: the memo copy fix (previous commit) still dropped the last byte when size == sizeof(memoBuf) exactly — the documented <=256 contract allows a full 256-byte raw memo, and copyLen was clamped to sizeof(memoBuf)-1 regardless. Widen memoBuf by one byte so the full 256-byte capacity can be copied verbatim while a terminator byte is still guaranteed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit b7d7c31)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 3d9be49)
…haped messages
Pioneer-powered Solana swaps reach the device as versioned (v0)
serialized messages and were always blind-signed:
- solana_parseVersionedTx forced every v0 message OPAQUE. Now a v0
message parses like a legacy one; only instructions that actually
reference address-lookup-table accounts (indices beyond the static
list, unresolvable on-device) force the tx opaque. An attached but
unreferenced lookup table does not block verification.
- Memo instruction bodies (THORChain swap intents '=:ETH.ETH:...') were
detected but hidden ('Memo attached'). Printable memos are now shown.
- SolanaSignMessage payloads that parse as a fully-verified Solana tx
(from byte 0) are clear-signed per instruction with the SignTx rules
(signer check + per-instruction confirms) instead of hex-blob blind
signing — this is the exact path wallet integrations use for v0 swap
txs. Unverifiable payloads keep the AdvancedMode gate + hex preview.
- Over-limit instruction counts now walk the section structurally, so
trailing-data and lookup-section checks still apply (truncated bodies
are malformed rather than opaque).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
(cherry picked from commit cc2d11f)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 4d69d6d)
…2's thorchain-only hunk)
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.
Dress-rehearsal PR 4/5 (stacks on #296). Applies the metadata/blind-sign gating to each non-EVM family. No thortx (owned by PR1a-#294). Local kkemu + firmware-unit green (52 per-chain tests).