diff --git a/Makefile b/Makefile index ec022536..5bfbd915 100644 --- a/Makefile +++ b/Makefile @@ -342,7 +342,7 @@ dmg: verify-arch test: test-zcash-cli test-unit test-unit: - cd $(PROJECT_DIR) && bun test __tests__/swap-parsing.test.ts __tests__/engine-state-machine.test.ts __tests__/device-switch.test.ts __tests__/wizard-messaging.test.ts __tests__/solana-tx.test.ts __tests__/solana-message-parser.test.ts __tests__/solana-instruction-decoder.test.ts __tests__/solana-alt.test.ts __tests__/solana-spl-decimals.test.ts __tests__/ton-build.test.ts __tests__/tron-memo-inject.test.ts __tests__/audit-coverage.test.ts __tests__/chain-scan.test.ts __tests__/recovery-ownership.test.ts src/bun/mcp.test.ts src/bun/txbuilder/hive-ops.test.ts + cd $(PROJECT_DIR) && bun test __tests__/evm-signer-verify.test.ts __tests__/swap-parsing.test.ts __tests__/engine-state-machine.test.ts __tests__/device-switch.test.ts __tests__/wizard-messaging.test.ts __tests__/solana-tx.test.ts __tests__/solana-message-parser.test.ts __tests__/solana-instruction-decoder.test.ts __tests__/solana-alt.test.ts __tests__/solana-spl-decimals.test.ts __tests__/ton-build.test.ts __tests__/tron-memo-inject.test.ts __tests__/audit-coverage.test.ts __tests__/chain-scan.test.ts __tests__/recovery-ownership.test.ts src/bun/mcp.test.ts src/bun/txbuilder/hive-ops.test.ts test-integration: test-rest diff --git a/projects/keepkey-vault/__tests__/evm-signer-verify.test.ts b/projects/keepkey-vault/__tests__/evm-signer-verify.test.ts new file mode 100644 index 00000000..16bc22ba --- /dev/null +++ b/projects/keepkey-vault/__tests__/evm-signer-verify.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, test } from 'bun:test' +import { ethers } from 'ethers' +import { EvmSignerVerificationError, verifyEvmSigner } from '../src/bun/evm-rpc' + +// Fixed key so the expectations below are stable; funds never touch this. +const TEST_KEY = '0x4c0883a69102937d6231471b5dbb6204fe5129617082792ae468d01a3f362318' +const wallet = new ethers.Wallet(TEST_KEY) + +/** A type-2 tx with calldata large enough to require multi-chunk transport. */ +async function signLargeCalldataTx(): Promise { + return wallet.signTransaction({ + to: '0x4c82d1fbfe28c977cbb58d8c7ff8fcf9f70a2cca', + value: 0, + nonce: 495, + gasLimit: ethers.BigNumber.from('0x6c8b8'), + maxFeePerGas: ethers.BigNumber.from('0x291d5740f'), + maxPriorityFeePerGas: ethers.BigNumber.from('0x218711a00'), + chainId: 1, + type: 2, + data: '0x3593564c' + '00'.repeat(1600), + }) +} + +describe('verifyEvmSigner', () => { + test('accepts a signature that recovers to the expected account', async () => { + const signed = await signLargeCalldataTx() + await expect(verifyEvmSigner(signed, wallet.address)).resolves.toBeUndefined() + }) + + test('accepts a checksum-mismatched expectation (case-insensitive compare)', async () => { + const signed = await signLargeCalldataTx() + await expect(verifyEvmSigner(signed, wallet.address.toLowerCase())).resolves.toBeUndefined() + }) + + test('rejects when the signature recovers to a different account', async () => { + // This is the firmware 7.x.0–7.14.0 EIP-1559 chunked-data failure in + // miniature: the bytes are a perfectly valid signature, they just do not + // belong to the account we asked to sign. Broadcasting is accepted by the + // RPC and then dropped from the mempool forever, so it must throw here. + const signed = await signLargeCalldataTx() + const someoneElse = ethers.Wallet.createRandom().address + + await expect(verifyEvmSigner(signed, someoneElse)).rejects.toBeInstanceOf(EvmSignerVerificationError) + await expect(verifyEvmSigner(signed, someoneElse)).rejects.toThrow(/recovered signer/i) + await expect(verifyEvmSigner(signed, someoneElse)).rejects.toThrow(/7\.14\.1/) + }) + + test('rejects unparseable bytes rather than letting them through', async () => { + await expect(verifyEvmSigner('0xdeadbeef', wallet.address)).rejects.toBeInstanceOf(EvmSignerVerificationError) + await expect(verifyEvmSigner('0xdeadbeef', wallet.address)).rejects.toThrow(/could not be parsed/i) + }) +}) diff --git a/projects/keepkey-vault/src/bun/evm-rpc.ts b/projects/keepkey-vault/src/bun/evm-rpc.ts index 17bad0f0..12298663 100644 --- a/projects/keepkey-vault/src/bun/evm-rpc.ts +++ b/projects/keepkey-vault/src/bun/evm-rpc.ts @@ -184,8 +184,65 @@ export async function estimateGas( } } -export async function broadcastEvmTx(rpcUrl: string, signedTxHex: string): Promise { +/** + * Assert that a device-signed transaction actually recovers to the account we + * asked the device to sign for. Throws (before any broadcast) when it doesn't. + * + * This is not paranoia about a compromised device — it catches signatures that + * are cryptographically valid over the *wrong pre-image*. KeepKey firmware + * 7.x.0 through 7.14.0 hashes the EIP-1559 empty access-list byte (0xC0) too + * early, so for any tx whose calldata exceeds the 1024-byte single-chunk limit + * the byte lands mid-stream instead of closing the RLP body. The resulting + * signature passes every check an RPC node makes, so the broadcast is + * ACCEPTED — and then the tx is dropped from the mempool forever, because the + * address it recovers to has no balance and the wrong nonce. Fixed in + * firmware 7.14.1; the stable release channel still ships an affected build, + * so the guard has to live here. + * + * Failing loudly before broadcast turns a silently-stuck transaction (and, on + * a swap, a spent approval with no swap) into an actionable error. + */ +export class EvmSignerVerificationError extends Error { + constructor(message: string) { + super(message) + this.name = 'EvmSignerVerificationError' + } +} + +export async function verifyEvmSigner(signedTxHex: string, expectedFrom: string): Promise { + const hex = signedTxHex.startsWith('0x') ? signedTxHex : `0x${signedTxHex}` + const { ethers } = await import('ethers') + + let recovered: string + try { + recovered = ethers.utils.parseTransaction(hex).from ?? '' + } catch (e: any) { + throw new EvmSignerVerificationError( + `Refusing to broadcast: the signed transaction could not be parsed (${e?.message ?? e}). ` + + `This usually means the device returned a malformed signature.`, + ) + } + + if (!recovered || recovered.toLowerCase() !== expectedFrom.toLowerCase()) { + throw new EvmSignerVerificationError( + `Refusing to broadcast: recovered signer ${recovered || '(none)'} ≠ expected ${expectedFrom}. ` + + `The signed bytes do not represent a transaction from your account. ` + + `If this transaction has large contract data, update your KeepKey to firmware 7.14.1 or later.`, + ) + } +} + +/** + * Broadcast a signed EVM transaction. + * + * `expectedFrom` is required on purpose: every caller here is broadcasting + * bytes that came back from the device, and a silently mis-signed tx is + * unrecoverable once it leaves. Making the parameter mandatory means a new + * broadcast path cannot forget the check — see verifyEvmSigner for why. + */ +export async function broadcastEvmTx(rpcUrl: string, signedTxHex: string, expectedFrom: string): Promise { const hex = signedTxHex.startsWith('0x') ? signedTxHex : `0x${signedTxHex}` + await verifyEvmSigner(hex, expectedFrom) const result = await ethRpc(rpcUrl, 'eth_sendRawTransaction', [hex]) return result } diff --git a/projects/keepkey-vault/src/bun/index.ts b/projects/keepkey-vault/src/bun/index.ts index a9a675ff..62b37c7a 100644 --- a/projects/keepkey-vault/src/bun/index.ts +++ b/projects/keepkey-vault/src/bun/index.ts @@ -4654,7 +4654,12 @@ const rpc = BrowserView.defineRPC({ if (rpcUrl) { const serialized = params.signedTx?.serializedTx || params.signedTx?.serialized || (typeof params.signedTx === 'string' ? params.signedTx : undefined) if (!serialized || typeof serialized !== 'string') throw new Error(`Cannot extract serialized tx from: ${JSON.stringify(params.signedTx).slice(0, 200)}`) - const txid = await broadcastEvmTx(rpcUrl, serialized) + // Build/sign/broadcast are separate RPC calls. The selected account + // can change between them, so verification must use the address that + // was attached to this signed request. + const expectedFrom = params.fromAddress + if (!expectedFrom) throw new Error('Cannot verify signer: signed EVM request has no fromAddress') + const txid = await broadcastEvmTx(rpcUrl, serialized, expectedFrom) result = { txid } } else { const pioneer = await getPioneer() diff --git a/projects/keepkey-vault/src/bun/swap.ts b/projects/keepkey-vault/src/bun/swap.ts index 81d395bc..7f9cd99e 100644 --- a/projects/keepkey-vault/src/bun/swap.ts +++ b/projects/keepkey-vault/src/bun/swap.ts @@ -16,7 +16,7 @@ import { findEvmSchema } from './evm-schema-registry' import { findSolanaSchema } from './solana-schema-registry' import { getPioneer } from './pioneer' import { encodeDepositWithExpiry, encodeApprove, parseUnits, toHex } from './txbuilder/evm' -import { getEvmGasPrice, getEvmFeeData, getEvmNonce, getEvmBalance, getErc20Allowance, getErc20Balance, getErc20Decimals, broadcastEvmTx, waitForTxReceipt, estimateGas } from './evm-rpc' +import { getEvmGasPrice, getEvmFeeData, getEvmNonce, getEvmBalance, getErc20Allowance, getErc20Balance, getErc20Decimals, broadcastEvmTx, EvmSignerVerificationError, waitForTxReceipt, estimateGas } from './evm-rpc' import * as txb from './txbuilder' import { normalizeBchAddress } from './txbuilder' // Re-export pure parsing functions (used by tests + this module) @@ -708,7 +708,7 @@ export async function executeSwap(params: ExecuteSwapParams, ctx: SwapContext): const rpcUrl = getRpcUrl(fromChain) stage('approve-broadcasting') if (rpcUrl) { - approvalTxid = await broadcastEvmTx(rpcUrl, approveHex) + approvalTxid = await broadcastEvmTx(rpcUrl, approveHex, fromAddress) swapLog(`${TAG} Relay-path approve broadcast: ${approvalTxid}`) stage('approve-waiting-receipt') const receipt = await waitForTxReceipt(rpcUrl, approvalTxid, 180_000) @@ -1017,9 +1017,14 @@ export async function executeSwap(params: ExecuteSwapParams, ctx: SwapContext): throw new Error(`Cannot extract serialized tx from signed result: ${JSON.stringify(signedTx).slice(0, 200)}`) } try { - txid = await broadcastEvmTx(swapRpcUrl, serializedHex) + txid = await broadcastEvmTx(swapRpcUrl, serializedHex, fromAddress) swapLog(`${TAG} Broadcast via direct RPC: ${txid}`) } catch (directErr: any) { + // A signer-verification failure is a local safety decision, not an RPC + // availability problem. Never route the rejected bytes around the guard + // through Pioneer. + if (directErr instanceof EvmSignerVerificationError) throw directErr + // The pre-sign balance check in buildRelaySwapTx already verified // value + gas <= native balance against this same RPC URL. So a node // "insufficient funds" here is NOT a real gas shortage — it's a stale @@ -1883,7 +1888,7 @@ async function buildEvmSwapTx( // Broadcast approve tx if (rpcUrl) { stage('approve-broadcasting') - approvalTxid = await broadcastEvmTx(rpcUrl, approveHex) + approvalTxid = await broadcastEvmTx(rpcUrl, approveHex, fromAddress) swapLog(`${TAG} Approve tx broadcast (direct RPC): ${approvalTxid}`) // Wait for approval receipt before building deposit — prevents nonce gap if approval reverts. diff --git a/projects/keepkey-vault/src/bun/walletconnect.ts b/projects/keepkey-vault/src/bun/walletconnect.ts index 594063d8..d906c777 100644 --- a/projects/keepkey-vault/src/bun/walletconnect.ts +++ b/projects/keepkey-vault/src/bun/walletconnect.ts @@ -13,6 +13,7 @@ import type { SessionTypes, SignClientTypes } from '@walletconnect/types' import bs58 from 'bs58' import type { SigningRequestInfo, WcSessionInfo } from '../shared/types' import { evmAddressPath } from './evm-addresses' +import { verifyEvmSigner } from './evm-rpc' import { parseSolanaTx } from './solana-tx' import { buildSolanaMessageDecodedInfo } from './solana-message-preview' @@ -916,6 +917,12 @@ export class WalletConnectManager { throw new Error('Device did not return serialized transaction') } + // Verify before the bytes leave the vault — whether we broadcast them or + // hand them back to the dApp to broadcast itself. A signature over a + // mangled pre-image is accepted by RPC nodes and then silently dropped + // from the mempool; see verifyEvmSigner in evm-rpc.ts. + await verifyEvmSigner(result.serialized, from) + if (broadcast) { const txHash = await this.rpcCall(effectiveChainId, 'eth_sendRawTransaction', [result.serialized]) return txHash diff --git a/projects/keepkey-vault/src/mainview/components/Dashboard.tsx b/projects/keepkey-vault/src/mainview/components/Dashboard.tsx index 6e1ccaee..e718b3f4 100644 --- a/projects/keepkey-vault/src/mainview/components/Dashboard.tsx +++ b/projects/keepkey-vault/src/mainview/components/Dashboard.tsx @@ -4,7 +4,7 @@ import { useTranslation } from "react-i18next" import { CHAINS, customChainToChainDef, isChainSupported, type ChainDef } from "../../shared/chains" import { isBitcoinOnlyVariant } from "../../shared/flags" import { versionCompare } from "../../shared/firmware-versions" -import { formatBalance } from "../lib/formatting" +import { formatBalance, hasNonZeroBalance } from "../lib/formatting" import { AnimatedUsd } from "./AnimatedUsd" import { getAssetIcon, registerCustomAsset } from "../../shared/assetLookup" import { AssetPage } from "./AssetPage" @@ -2534,9 +2534,15 @@ export function Dashboard({ onLoaded, watchOnly, watchOnlyDeviceId, onOpenSettin const agg = balances.get(drilledChainId) const isEvm = dchain.chainFamily === 'evm' const fundedAddrs = isEvm - ? evmAddressSet.addresses.filter(a => (a.chainBalances?.[drilledChainId]?.balanceUsd ?? 0) > 0) + ? evmAddressSet.addresses.filter(a => { + const cb = a.chainBalances?.[drilledChainId] + return (cb?.balanceUsd ?? 0) > 0 || hasNonZeroBalance(cb?.balance) + }) : [] - const hasAggregate = (agg?.balanceUsd ?? 0) > 0 + // Chains whose price feed is missing (e.g. Arbitrum native ETH) + // report balanceUsd 0 while still holding funds. Gate on the + // token amount so held funds never render as "no balance". + const hasAggregate = (agg?.balanceUsd ?? 0) > 0 || hasNonZeroBalance(agg?.balance) return ( {deriveError || "Couldn't derive your Hive key from the device."} - {onRetryDerive && } + {onRetryDerive && } ) return @@ -104,7 +119,7 @@ export function HiveAccountPanel({ activeKey, color, loading, deriveError, onRet if (state === "error") return ( Couldn't reach the Hive account service. Try again shortly. - + ) @@ -128,9 +143,9 @@ export function HiveAccountPanel({ activeKey, color, loading, deriveError, onRet { label: "Hive.blog", url: `https://hive.blog/@${account.name}` }, { label: "Ecency", url: `https://ecency.com/@${account.name}` }, ].map(l => ( - ))} diff --git a/projects/keepkey-vault/src/mainview/components/device/SigningApproval.tsx b/projects/keepkey-vault/src/mainview/components/device/SigningApproval.tsx index 864641c7..ba359b6d 100644 --- a/projects/keepkey-vault/src/mainview/components/device/SigningApproval.tsx +++ b/projects/keepkey-vault/src/mainview/components/device/SigningApproval.tsx @@ -41,9 +41,13 @@ const METHOD_LABEL_KEYS: Record = { } const SIGNING_ANIMATIONS = ` + /* Deliberately restrained: a signing prompt should read as serious, not + alarming. The old 24px/48px throb washed the card edge out and made the + hex payload hard to scan. This keeps a faint gold presence that breathes + instead of pulsing. */ @keyframes signingPulseGlow { - 0%, 100% { box-shadow: 0 0 8px 2px rgba(233,196,106,0.4); } - 50% { box-shadow: 0 0 24px 8px rgba(233,196,106,0.7), 0 0 48px 16px rgba(233,196,106,0.15); } + 0%, 100% { box-shadow: 0 0 0 1px rgba(233,196,106,0.10), 0 16px 44px -12px rgba(0,0,0,0.75); } + 50% { box-shadow: 0 0 10px 1px rgba(233,196,106,0.18), 0 16px 44px -12px rgba(0,0,0,0.75); } } @keyframes signingFlashBorder { 0%, 100% { border-color: rgba(233,196,106,0.5); } @@ -372,6 +376,127 @@ function CalldataSection({ decoded, t }: { decoded: CalldataDecodedInfo; t: (k: ) } +// ── Raw calldata inspector (Etherscan-style hex / decoded toggle) ───── + +type CalldataWord = { index: number; hex: string; asAddress?: string; asUint?: string } + +/** + * ABI-less calldata split, mirroring Etherscan's "Decode Input Data" default + * view. Without an ABI we cannot name parameters, but the 4-byte selector plus + * indexed 32-byte words is exactly what makes a blind-signing payload auditable: + * a reviewer can spot recipient addresses and amounts in the argument slots. + * + * Heuristics per word (both may be shown; neither is authoritative): + * - 12 leading zero bytes + 20 non-zero bytes -> likely an address + * - fits in a JS-safe integer -> show the decimal value + * + * Dynamic types (offsets, arrays, bytes) still appear as words, matching how + * Etherscan renders them when no ABI is available. + */ +function decodeRawCalldata(data: string): { selector: string; words: CalldataWord[]; trailing?: string } | null { + const hex = data.startsWith("0x") || data.startsWith("0X") ? data.slice(2) : data + if (!/^[0-9a-fA-F]*$/.test(hex) || hex.length < 8) return null + + const selector = "0x" + hex.slice(0, 8) + const body = hex.slice(8) + const words: CalldataWord[] = [] + const fullWords = Math.floor(body.length / 64) + + for (let i = 0; i < fullWords; i++) { + const word = body.slice(i * 64, i * 64 + 64) + const w: CalldataWord = { index: i, hex: "0x" + word } + if (/^0{24}/.test(word) && !/^0{64}$/.test(word)) { + w.asAddress = "0x" + word.slice(24) + } + // Only surface a decimal when it round-trips exactly — a truncated + // big number is worse than no number at all. + const asBig = BigInt("0x" + word) + if (asBig <= BigInt(Number.MAX_SAFE_INTEGER)) w.asUint = asBig.toString() + words.push(w) + } + + const rest = body.slice(fullWords * 64) + return { selector, words, trailing: rest.length ? "0x" + rest : undefined } +} + +function CalldataInspector({ data, t }: { data: string; t: (k: string, f?: string) => string }) { + const [view, setView] = useState<"hex" | "decoded">("hex") + const parsed = view === "decoded" ? decodeRawCalldata(data) : null + + return ( + + + + {t("signing.data", "Data")} + + + {(["hex", "decoded"] as const).map(mode => ( + + ))} + + + + + {view === "hex" || !parsed ? ( + + {view === "decoded" && !parsed + ? t("signing.dataNotDecodable", "Payload is not valid hex calldata — showing raw value.") + " " + data + : data} + + ) : ( + + + + {t("signing.selector", "Selector")} + + {parsed.selector} + + {parsed.words.map(w => ( + + [{w.index}] + + {w.hex} + {(w.asAddress || w.asUint) && ( + + {w.asAddress && ( + + {t("signing.asAddress", "addr")}: {w.asAddress} + + )} + {w.asUint && ( + + {t("signing.asUint", "uint")}: {w.asUint} + + )} + + )} + + + ))} + {parsed.trailing && ( + + + {t("signing.trailing", "extra")} + + {parsed.trailing} + + )} + + )} + + + ) +} + // ── Solana decoded section ──────────────────────────────────────────── function shortenPubkey(pk: string): string { @@ -740,9 +865,9 @@ export function SigningApproval({ request, phase, onApprove, onReject, onCancel > {/* ── Header row: badge + app + method + timer + trust ── */} - + + {/* Scroll region. Only the reviewable content scrolls — the header + above and the action buttons below stay pinned, so a large + calldata blob can never push Approve/Reject out of reach. + minH=0 is required for a flex child to shrink below its + content height and actually scroll. */} + {/* ── Method ── */} - {methodLabel} + {methodLabel} {/* ── AdvancedMode gate ── */} {advancedModeRequired && ( @@ -957,7 +1088,7 @@ export function SigningApproval({ request, phase, onApprove, onReject, onCancel {request.chainId !== undefined && } {request.data && (!decoded || decoded.source === 'none') && ( - + )} @@ -966,9 +1097,10 @@ export function SigningApproval({ request, phase, onApprove, onReject, onCancel {/* ── Full raw payload (collapsible) ── */} + - {/* ── Action buttons ── */} - + {/* ── Action buttons (pinned below the scroll region) ── */} +