From e2087db17c64006a8e404e48eda6e891808486ab Mon Sep 17 00:00:00 2001 From: Deepak Jangid Date: Thu, 5 Nov 2026 16:22:22 +0530 Subject: [PATCH] fix(abstract-eth): verify EVM TSS intent before signing Compare payment and compatible consolidation transactions with their authorized intent, and reject definite mismatches before signing. Bind serialized legacy transactions to the signing preimage, preserve specialized formats, and warn when trusted metadata is unavailable. Ticket: CECHO-2460 --- modules/abstract-eth/CHANGELOG.md | 11 + modules/abstract-eth/README.md | 75 + .../src/abstractEthLikeNewCoins.ts | 1492 +++++++++++++++-- modules/abstract-eth/test/unit/index.ts | 1 + .../abstract-eth/test/unit/tssVerification.ts | 1360 +++++++++++++++ modules/sdk-coin-bsc/test/unit/bsc.ts | 2 +- .../test/unit/tssSigningVerification.ts | 390 +++++ modules/sdk-coin-eth/test/unit/eth.ts | 213 ++- .../test/unit/tssMinimalVerification.ts | 928 ++++++++++ modules/sdk-coin-evm/CHANGELOG.md | 6 + modules/sdk-coin-evm/src/evmCoin.ts | 43 - .../test/unit/tssSigningVerification.ts | 645 +++++++ modules/sdk-coin-flr/src/flr.ts | 104 ++ modules/sdk-coin-flr/test/resources.ts | 8 + modules/sdk-coin-flr/test/unit/flr.ts | 239 ++- modules/sdk-coin-xdc/test/unit/xdc.ts | 2 +- modules/sdk-coin-xdc/test/unit/xdcToken.ts | 4 +- modules/sdk-core/CHANGELOG.md | 6 + .../sdk-core/src/bitgo/baseCoin/iBaseCoin.ts | 20 + .../sdk-core/src/bitgo/utils/tss/baseTypes.ts | 2 + .../src/bitgo/utils/tss/ecdsa/ecdsa.ts | 31 +- .../src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts | 38 +- .../src/bitgo/utils/tss/recipientUtils.ts | 6 +- .../bitgo/utils/tss/serializedTxHexVerify.ts | 153 +- .../unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts | 3 + .../unit/bitgo/utils/tss/recipientUtils.ts | 36 + .../bitgo/utils/tss/serializedTxHexVerify.ts | 117 ++ modules/statics/CHANGELOG.md | 7 + modules/statics/src/allCoinsAndTokens.ts | 4 + modules/statics/src/networks.ts | 2 + modules/statics/test/unit/coins.ts | 13 + modules/statics/test/unit/networks.ts | 9 + 32 files changed, 5696 insertions(+), 274 deletions(-) create mode 100644 modules/abstract-eth/test/unit/tssVerification.ts create mode 100644 modules/sdk-coin-bsc/test/unit/tssSigningVerification.ts create mode 100644 modules/sdk-coin-eth/test/unit/tssMinimalVerification.ts create mode 100644 modules/sdk-coin-evm/test/unit/tssSigningVerification.ts create mode 100644 modules/sdk-core/test/unit/bitgo/utils/tss/serializedTxHexVerify.ts diff --git a/modules/abstract-eth/CHANGELOG.md b/modules/abstract-eth/CHANGELOG.md index 5ce0ae94ce4..5f56ca72bf1 100644 --- a/modules/abstract-eth/CHANGELOG.md +++ b/modules/abstract-eth/CHANGELOG.md @@ -3,6 +3,17 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## Unreleased + +### Bug Fixes + +* Reject EVM TSS payments whose decoded destination, amount, token contract, native value or calldata shape does not match the declared intent, including on the pending-approval path where the stored `payment` intent previously skipped the `transfer` checks. +* Verify batch payments against the network batcher and the full decoded recipient/amount arrays. +* Never let server data select the TSS intent handler, supply absent caller calldata or replace a declared token; `callerTxParams` and the stored intent are now kept apart. +* Reject `contractCall` / `customTx` on EVM TSS and reject malformed EVM bytes for every intent type; intent types without verification rules are reported explicitly instead of passing silently. +* Bind legacy signing payloads to the coin's own network configuration and the transaction envelope rather than a chain-id lookup. +* Verify EVM TSS consolidations: recognise the stored `consolidateToken` intent even without a consolidate id (the missing-params guard is unchanged — recipients or a consolidate id are still required), require a consolidation whose intent declares the chain asset to carry no calldata, and require token consolidations to use canonical ERC-20 `transfer(address,uint256)` calldata paying the base address with zero native value (any other function on an established ERC-20 contract, and native value on an established ERC-20 sweep, are rejected). An authorised field naming the chain asset makes any calldata a definite mismatch, a token contract statics classifies as ERC-20 must be swept with `transfer(address,uint256)` (batch encodings included), and the flag-driven path keeps its historic rejection: a tx that is not an ERC-20 transfer must pay the base address with `tx.to`. A generic `consolidate` that declares no asset is not assumed native — its batch and arbitrary shapes are reported, as are batches of an unresolved token and shapes on a specialised (ERC-7984) token verified by its own coin. A declared asset only counts when it resolves on the transaction's own network: a name from another chain (including non-EVM coins and tokens) is treated as unresolved and reported, never as this chain's native asset or as a local contract. ERC-20 expectations apply only on positive evidence: exported ERC-20 classes by identity and the unexported BSC/XDC/L2/generic ERC-20 families by a verified class allowlist; any other token class (NFT, specialised, future type, minified name) stays unclassified and is reported. The token contract is compared only when the intent declares a token identity that resolves through statics to exactly one EVM contract on this network; otherwise the base address, calldata shape and zero native value are still enforced and a warning names the unchecked token contract instead of claiming it was verified. Sweep amounts are never compared with an intent amount. + # [25.6.0](https://github.com/BitGo/BitGoJS/compare/%40bitgo%2Fabstract-eth%4025.5.4...%40bitgo%2Fabstract-eth%4025.6.0) (2026-11-03) ### Bug Fixes diff --git a/modules/abstract-eth/README.md b/modules/abstract-eth/README.md index 856c0af47ca..489612fd65d 100644 --- a/modules/abstract-eth/README.md +++ b/modules/abstract-eth/README.md @@ -1 +1,76 @@ # BitGo abstract-eth + +## EVM TSS transaction verification + +`verifyTssTransaction` decodes the EVM bytes about to be signed and compares `to` / `value` / +`data` with the declared intent before the user key share signs (TOB-BITGOEDMPC-2 / +INFOSEC-671). Verification runs in one mode: a definite mismatch throws +`TxIntentMismatchRecipientError`; a check the verifier cannot perform is reported through +`console.warn('[tss verification] ...')` and signing continues. + +- The caller's own `txParams` are authoritative whenever they declare `type` or `recipients`. The + stored tx request intent is consulted only when the caller declared nothing (pending-approval + signing). Server data never selects the handler, supplies absent caller calldata, or replaces a + declared token. `payment` and `transfer` are the same intent; `transferToken` is not a payment + and keeps its own specialised handling. +- A payment must match a closed set of shapes: a native send (`to` / `value`, empty data), an + ERC-20 `transfer(address,uint256)` to the declared token contract with zero native value, an + ERC-7984 `confidentialTransfer` (recipient checked, encrypted amount cannot be), or a batcher + batch. A recipient that declares its own calldata (WalletConnect) is verified against that exact + `to` / `value` / `data`, nonzero native value included. Batch payments must target the network + batcher and match the decoded recipient/amount arrays 1:1, with `value` equal to the sum + (native) or zero (token). +- Token identity comes only from locally trusted declarations: recipient `tokenAddress` / + `tokenName` / `tokenData`, the top-level `tokenName`, and the coin instance. A declaration only + counts when it resolves on the transaction's own network: a name that resolves on another chain + (including non-EVM coins and tokens such as `sol:usdc`) is never read as this chain's native + asset or as a local contract, and the asset stays unresolved so the missing check is reported. + Identity that cannot be resolved to an EVM contract, or that names two assets, is reported the + same way, and the checks that do not depend on it are still enforced. Calldata is compared in + canonical form and the supported shapes reject trailing bytes. +- A consolidation is recognised from the `consolidate` / `consolidateToken` intent type, a + consolidate id, or the caller's `verification.consolidationToBaseAddress` flag; a token + consolidation is recognised from its intent type even when the consolidate id is absent, while + the missing-params guard is unchanged — the call still needs recipients or a consolidate id. + A native consolidation is a plain send to the wallet base address and must carry no calldata. + Native counts as *declared* only when an authorised field names the chain asset; a generic + `consolidate` that declares no asset is not assumed native, because the same type carries + ERC-20 and specialised sweeps. ERC-20 expectations are applied only on positive evidence: the + exported ERC-20 families (`Erc20Coin`, `CeloCoin`, `AvaxERC20Token`) matched by class identity, + plus the unexported BSC/XDC/L2/generic ERC-20 families matched by a verified class allowlist. + Any other token class — an NFT, a specialised standard, a future type, or a name a bundler + rewrote — stays unclassified and is reported, never assumed to be ERC-20; ERC-7984 tokens are + verified by their own coin. A token consolidation on an ERC-20 contract must + be exactly `transfer(baseAddress, amount)` — canonical calldata, no trailing bytes — paying + the base address with zero native value; any other function, batch encodings included, is + rejected, and native value on an ERC-20 sweep is rejected. How a specialised or unclassified + standard behaves with native value is not established, so it is reported instead. + Shapes this verifier does not model are reported, not rejected: a batch or arbitrary call with no + asset declared, non-`transfer` calldata while the token contract is unresolved, and any shape on + a specialised (ERC-7984) token, whose consolidation is verified by its own coin. The flag-driven + path also keeps its historic rule that a tx which is not an ERC-20 transfer must pay the wallet + base address with `tx.to`; when `to` *is* the base address, the bytes are otherwise checked + against the asset declaration, reported, or rejected as above. The token contract is compared + with the identity the intent declares, taken from the caller when it declared the intent and + otherwise from the stored intent (`tokenData.tokenName` / `tokenContractAddress`, recipient + `tokenAddress`), and **only when that declaration exists and resolves through statics to exactly + one EVM contract**. Without such a declaration the base address, the calldata shape and the zero + native value are still enforced, and a warning names the token contract as unchecked: an + unresolved declaration is never treated as a verified contract, so no consolidation path claims + more than it proved. Consolidations never compare amounts — a sweep moves whatever the address + holds, so the calldata amount is not verified against any intent amount. +- Missing or malformed transaction bytes always fail, including for intent types without intent + rules and for consolidations whose base address is unavailable. For types without rules the EVM + encoding is validated first, then a warning states that the bytes were not compared against the + intent. Non-EVM formats are owned by the coin that supports them, never sniffed here: the `Flr` + override recognises FLR's atomic cross-chain bytes (export, import, importtoc) with the FLR + parser and warns that they were not verified against the intent, and Tempo / VET override + `verifyTransaction`. Any payload that reaches this verifier must decode as an EVM transaction, + so an unrecognised non-EVM payload is rejected. `contractCall` and `customTx` always reject. +- Not implemented and therefore reported, not verified: `transferToken` / NFT payments, + `tokenApproval`, `fillNonce`, staking intents, `acceleration`, `bridgeFunds`, `enableToken` / + `disableToken`, DeFi redeem, consolidations whose intent declares no asset (batch and arbitrary + calldata), batches of an unresolved token, and stored DeFi approve/deposit/withdraw legs + (no locally trusted vault registry anchors their target or spender). FLR atomic imports are + handled in the `Flr` override and never reach this verifier. ERC-7984 `confidentialTransfer` + amounts are encrypted and cannot be compared. diff --git a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts index ff45cad8def..46f8431d614 100644 --- a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts +++ b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts @@ -28,6 +28,7 @@ import { MPCTxs, ParsedTransaction, ParseTransactionOptions, + PopulatedIntent, PrebuildTransactionResult, PresignTransactionOptions as BasePresignTransactionOptions, Recipient, @@ -58,10 +59,14 @@ import { import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; import { bip32 } from '@bitgo/secp256k1'; import { + AvaxERC20Token, BaseCoin as StaticsBaseCoin, + CeloCoin, ChainIdNotFoundError, CoinFeature, coins, + ContractAddressDefinedToken, + Erc20Coin, Erc7984Coin, EthereumNetwork as EthLikeNetwork, ethGasConfigs, @@ -86,6 +91,10 @@ import { calculateForwarderV1Address, coinFamiliesWithL1Fees, approveMethodId, + confidentialTransferNoProofMethodId, + confidentialTransferNoProofTypes, + confidentialTransferWithProofMethodId, + confidentialTransferWithProofTypes, decodeApproveCalldata, decodeFinalizeUnwrapCalldata, decodeNativeTransferData, @@ -412,6 +421,11 @@ const MAX_UINT256 = 2n ** 256n - 1n; export interface VerifyEthTransactionOptions extends VerifyTransactionOptions { txPrebuild: TransactionPrebuild; txParams: EthTransactionParams; + /** + * EVM callers declare the asset they are paying with as a top-level `tokenName`, which the + * chain-agnostic `TransactionParams` does not carry. + */ + callerTxParams?: EthTransactionParams; } interface PresignTransactionOptions extends TransactionPrebuild, BasePresignTransactionOptions { @@ -588,6 +602,575 @@ export const optionalDeps = { }, }; +/** Canonical intent types that must never reach signing on an EVM chain. */ +const EVM_TSS_UNSUPPORTED_INTENT_TYPES = new Set(['contractCall', 'customTx']); + +/** + * Intent types that already have dedicated verification: the DeFi vault legs and ERC-7984 + * shield/unshield legs checked in this file or in the token coin. They are excluded from the + * "no fail-closed verifier" warning; every other type without a handler is reported. + */ +const EVM_TSS_DEDICATED_CHECK_INTENT_TYPES = new Set([ + 'defiApprove', + 'defiDeposit', + 'defiWithdraw', + 'wrapApprove', + 'wrap', + 'unwrap', + 'finalizeUnwrap', +]); + +/** Stored (kebab-case) intent spellings mapped onto the canonical verifier type. */ +const EVM_TSS_INTENT_TYPE_ALIASES: Record = { + payment: 'transfer', + 'transfer-token': 'transferToken', + transfertoken: 'transferToken', + 'contract-call': 'contractCall', + 'custom-tx': 'customTx', + 'defi-approve': 'defiApprove', + 'defi-deposit': 'defiDeposit', + 'defi-withdraw': 'defiWithdraw', + 'wrap-approve': 'wrapApprove', + // Token consolidation; WP persists the camelCase spelling, the kebab form is accepted too. + 'consolidate-token': 'consolidateToken', +}; + +/** ERC-20 `transfer(address,uint256)` selector. */ +const ERC20_TRANSFER_METHOD_ID = '0xa9059cbb'; + +/** + * A consolidation has exactly two shapes. Calldata that fits neither is a definite mismatch, never + * an unverifiable shape: the unknown half of an unresolved token is its contract, not its calldata. + */ +const TSS_CONSOLIDATION_TOKEN_SHAPE_MESSAGE = + 'Consolidation transaction does not use ERC-20 transfer(address,uint256): the declared token cannot be swept ' + + 'to the wallet base address'; +const TSS_CONSOLIDATION_NATIVE_CALLDATA_MESSAGE = + 'Consolidation transaction must not carry calldata: a native consolidation is a plain send to the wallet ' + + 'base address'; + +/** A well-formed 20-byte EVM address. Anything else cannot name a token contract. */ +const EVM_ADDRESS_PATTERN = /^0x[0-9a-fA-F]{40}$/; + +/** One recipient of the user intent, flattened from txParams or the stored tx request intent. */ +interface TssExpectedRecipient { + address: string; + amount: string; + /** Raw calldata the intent declares for this recipient (WalletConnect / dApp flows). */ + data?: string; + tokenName?: string; + tokenAddress?: string; + tokenData?: { + tokenName?: string; + tokenContractAddress?: string; + tokenType?: string; + }; +} + +/** The asset a payment is declared to move, resolved only from locally declared identity. */ +type TssPaymentAsset = + | { kind: 'native' } + | { kind: 'token'; contract: string } + | { kind: 'unresolved'; reason: string }; + +/** The `to` / `value` / `data` of the bytes about to be signed. */ +interface DecodedTssTransaction { + to: string; + value: string; + data: string; +} + +type TssMismatchThrower = (message: string, mismatchedRecipients: Recipient[]) => Promise; + +/** + * Report a check this verifier could not perform. A warning names the intent type and the missing + * check only: expected and decoded transaction fields (addresses, values, calldata) never reach the + * logs, where they would outlive the signing attempt. + */ +type TssUnverifiableReporter = (intentType: string, reason: string) => void; + +/** + * Map a declared transaction type onto the canonical verifier type. Caller `txParams.type` and + * the stored `intent.intentType` both flow through here, so `payment` lands on `transfer` and + * kebab-case stored spellings (`defi-approve`, `wrap-approve`, …) on their handlers. + */ +function canonicalTssIntentType(rawType?: string): string { + if (!rawType) { + return ''; + } + const trimmed = rawType.trim(); + return ( + EVM_TSS_INTENT_TYPE_ALIASES[trimmed] ?? + EVM_TSS_INTENT_TYPE_ALIASES[trimmed.toLowerCase()] ?? + trimmed.replace(/-([a-z0-9])/g, (_, char: string) => char.toUpperCase()) + ); +} + +/** + * Canonical calldata for every comparison: missing data, `''` and `'0x'` are the same empty + * payload, hex is lowercased, and a dropped `0x` prefix is restored. + */ +function normalizeTssCalldata(data?: string): string { + const lower = (data ?? '').toLowerCase(); + if (!lower || lower === '0x') { + return '0x'; + } + return lower.startsWith('0x') ? lower : `0x${lower}`; +} + +function isTssEmptyCalldata(data?: string): boolean { + return normalizeTssCalldata(data) === '0x'; +} + +function normalizeTssAddress(address?: string): string { + return (address ?? '').toLowerCase(); +} + +function tssValuesEqual(a: unknown, b: unknown): boolean { + try { + return BigInt(String(a)) === BigInt(String(b)); + } catch { + return String(a) === String(b); + } +} + +/** `getRawDecoded` keeps uint256 values as BN and addresses / bytes as byte arrays. */ +function tssToBuffer(value: unknown): Buffer { + return Buffer.isBuffer(value) ? value : Buffer.from(value as Uint8Array); +} + +function tssBigIntFromBuffer(value: unknown): bigint { + // `ethereumjs-abi` returns uint256 as BN, and address / bytes32 / bytes as hex strings or + // byte arrays depending on the type, so all three shapes are accepted here. + if (BN.isBN(value)) { + return BigInt((value as BN).toString(10)); + } + if (typeof value === 'string') { + return BigInt(addHexPrefix(value)); + } + const hex = bufferToHex(tssToBuffer(value)); + return BigInt(hex === '0x' ? '0' : hex); +} + +function tssDecodedAddress(value: unknown): string { + if (typeof value === 'string') { + return addHexPrefix(value).toLowerCase(); + } + return addHexPrefix(bufferToHex(tssToBuffer(value))).toLowerCase(); +} + +/** The ABI payload of selector-prefixed calldata, or a throw when it is not valid hex. */ +function tssCalldataPayload(calldata: string): Buffer { + const payloadHex = normalizeTssCalldata(calldata).slice(10); + if (payloadHex.length % 2 !== 0 || /[^0-9a-f]/.test(payloadHex)) { + throw new Error(`transaction calldata is not valid hex: ${calldata}`); + } + return Buffer.from(payloadHex, 'hex'); +} + +function readTssAbiWord(payloadHex: string, wordIndex: number): bigint { + const start = wordIndex * 64; + if (start + 64 > payloadHex.length) { + throw new Error(`transaction calldata is truncated at ABI word ${wordIndex}`); + } + return BigInt(`0x${payloadHex.slice(start, start + 64)}`); +} + +/** Reject calldata whose payload is not exactly `wordCount` 32-byte ABI words. */ +function assertCanonicalStaticTssArgs(calldata: string, wordCount: number, label: string): void { + const payloadHex = normalizeTssCalldata(calldata).slice(10); + if (payloadHex.length !== wordCount * 64) { + throw new Error( + `${label} calldata is not canonical: ${payloadHex.length / 2} payload bytes, expected ${wordCount * 32}` + ); + } +} + +/** + * Reject calldata whose final dynamic `bytes` argument is not the canonical encoding: its + * offset must point past the head, and a single length word plus exactly the padded payload + * must be the rest of the calldata. + */ +function assertCanonicalBytesTailTssArgs(calldata: string, dynamicArgIndex: number, label: string): void { + const payloadHex = normalizeTssCalldata(calldata).slice(10); + const headBytes = (dynamicArgIndex + 1) * 32; + const offset = readTssAbiWord(payloadHex, dynamicArgIndex); + if (offset !== BigInt(headBytes)) { + throw new Error(`${label} calldata is not canonical: the bytes argument offset is ${offset}`); + } + const length = readTssAbiWord(payloadHex, dynamicArgIndex + 1); + if (length > BigInt(payloadHex.length / 2)) { + throw new Error(`${label} calldata is not canonical: the bytes argument length ${length} exceeds the payload`); + } + const paddedLength = Number((length + 31n) / 32n) * 32; + const expected = headBytes * 2 + 64 + paddedLength * 2; + if (payloadHex.length !== expected) { + throw new Error( + `${label} calldata is not canonical: ${payloadHex.length / 2} payload bytes, expected ${expected / 2}` + ); + } +} + +interface DecodedTssBatch { + tokenContract?: string; + recipients: Array<{ address: string; amount: string }>; +} + +/** + * Decode the batcher's `batch(address[],uint256[])` / `batchTransferFrom(address,address[],uint256[])` + * calldata and reject anything that is not its canonical encoding (head offsets, array lengths, + * total size). Decoding never splits on the selector: an argument word may contain those bytes. + */ +function decodeTssBatchCalldata(calldata: string, tokenBatch: boolean): DecodedTssBatch { + const payloadHex = normalizeTssCalldata(calldata).slice(10); + if (payloadHex.length % 64 !== 0) { + throw new Error('batch calldata is not canonical: the payload is not a whole number of ABI words'); + } + const types = tokenBatch ? ['address', 'address[]', 'uint256[]'] : ['address[]', 'uint256[]']; + const decoded = getRawDecoded(types, Buffer.from(payloadHex, 'hex')) as unknown[]; + const headWords = tokenBatch ? 3 : 2; + const addressOffsetWord = tokenBatch ? 1 : 0; + const addresses = decoded[tokenBatch ? 1 : 0]; + const amounts = decoded[tokenBatch ? 2 : 1]; + if (!Array.isArray(addresses) || !Array.isArray(amounts)) { + throw new Error('batch calldata is not canonical: address and amount arguments must be arrays'); + } + if (addresses.length !== amounts.length) { + throw new Error(`Mismatched batch address/amount array lengths: ${addresses.length} vs ${amounts.length}`); + } + const headBytes = headWords * 32; + if (readTssAbiWord(payloadHex, addressOffsetWord) !== BigInt(headBytes)) { + throw new Error('batch calldata is not canonical: the address array offset is not the first tail word'); + } + const amountOffsetWord = addressOffsetWord + 1; + if (readTssAbiWord(payloadHex, amountOffsetWord) !== BigInt(headBytes + 32 * (1 + addresses.length))) { + throw new Error('batch calldata is not canonical: the amount array offset does not follow the address array'); + } + const expectedBytes = headBytes + 32 * (1 + addresses.length) + 32 * (1 + amounts.length); + if (payloadHex.length !== expectedBytes * 2) { + throw new Error( + `batch calldata is not canonical: ${payloadHex.length / 2} payload bytes, expected ${expectedBytes}` + ); + } + return { + ...(tokenBatch && { tokenContract: tssDecodedAddress(decoded[0]) }), + recipients: addresses.map((address, index) => ({ + address: tssDecodedAddress(address), + amount: tssBigIntFromBuffer(amounts[index]).toString(), + })), + }; +} + +/** + * True only for the intent `tokenData.tokenType` values that denote an NFT. Ordinary ERC-20 + * recipients also carry `tokenType` (e.g. `ERC20`) plus `tokenQuantity` / `tokenName`, so this + * must never be a generic "has tokenData" check. + */ +function isNftTokenType(tokenType?: string): boolean { + const normalized = tokenType?.trim().toUpperCase(); + return normalized === 'ERC721' || normalized === 'ERC1155'; +} + +/** Calldata selectors of the network batcher's batch functions. */ +function tssBatchMethodIds(): { native: string; token: string } { + return { + native: addHexPrefix( + optionalDeps.ethAbi.methodID('batch', ['address[]', 'uint256[]']).toString('hex') + ).toLowerCase(), + token: addHexPrefix( + optionalDeps.ethAbi.methodID('batchTransferFrom', ['address', 'address[]', 'uint256[]']).toString('hex') + ).toLowerCase(), + }; +} + +type TssDeclaredRecipient = NonNullable[number]; + +/** + * Read the recipients of the stored tx request intent in the same shape as caller recipients. + * Used only when the caller declared nothing and the effective params did not carry them. + */ +function tssRecipientsFromStoredIntent(intent: PopulatedIntent): TssDeclaredRecipient[] { + return (intent.recipients ?? []).map((recipient) => ({ + address: recipient.address.address, + amount: String(recipient.amount.value), + ...(recipient.data !== undefined && { data: recipient.data }), + ...(recipient.tokenData?.tokenName !== undefined && { tokenName: recipient.tokenData.tokenName }), + })); +} + +/** + * Flatten the recipients of the source that speaks for the user. The stored intent only fills + * in the `tokenData` shape the effective txParams do not carry; it never supplies a recipient. + */ +function toTssExpectedRecipients( + recipients: TssDeclaredRecipient[], + storedIntent?: PopulatedIntent +): TssExpectedRecipient[] { + return recipients.map((recipient, index) => { + const callerTokenData = (recipient as TssDeclaredRecipient & { tokenData?: TssExpectedRecipient['tokenData'] }) + .tokenData; + const tokenData = callerTokenData ?? storedIntent?.recipients?.[index]?.tokenData; + return { + address: recipient.address, + amount: String(recipient.amount), + ...(recipient.data !== undefined && { data: recipient.data }), + ...(recipient.tokenName !== undefined && { tokenName: recipient.tokenName }), + ...(recipient.tokenAddress !== undefined && { tokenAddress: recipient.tokenAddress }), + ...(tokenData && { + tokenData: { + ...(tokenData.tokenName !== undefined && { tokenName: tokenData.tokenName }), + ...(tokenData.tokenContractAddress !== undefined && { + tokenContractAddress: tokenData.tokenContractAddress, + }), + ...(tokenData.tokenType !== undefined && { tokenType: tokenData.tokenType }), + }, + }), + }; + }); +} + +/** + * ERC-20-compatible statics token classes that statics does not export, recognised by their + * verified class name. This is a positive allowlist: a class that is not named here — an NFT, a + * future token type, or a class whose name a bundler rewrote — stays unclassified and is reported, + * never assumed to be ERC-20. + */ +const ERC20_TOKEN_CLASS_NAMES: Record = { + BscCoin: true, + EthLikeERC20Token: true, + PolygonERC20Token: true, + ArbethERC20Token: true, + OpethERC20Token: true, + ZkethERC20Token: true, + BeraERC20Token: true, + CoredaoERC20Token: true, + WorldERC20Token: true, + FlrERC20Token: true, + XdcERC20Token: true, + MonERC20Token: true, +}; + +/** + * The standard statics establishes for a contract-defined token, judged only on positive evidence: + * `Erc7984Coin` is a specialised standard; `Erc20Coin`, `CeloCoin` and `AvaxERC20Token` are + * ERC-20-compatible and export their classes, so they are matched by identity; the remaining + * ERC-20-compatible families (`BscCoin`, `XdcERC20Token`, the L2 token classes, …) are matched by + * the allowlist above. Everything else is unclassified, which makes the verifier report the shape + * instead of applying ERC-20 expectations to it. + */ +function classifyTssTokenStandard(coin: ContractAddressDefinedToken): 'erc20' | 'specialized' | undefined { + if (coin instanceof Erc7984Coin) { + return 'specialized'; + } + if (coin instanceof Erc20Coin || coin instanceof CeloCoin || coin instanceof AvaxERC20Token) { + return 'erc20'; + } + return ERC20_TOKEN_CLASS_NAMES[coin.constructor.name] ? 'erc20' : undefined; +} + +/** One locally trusted asset declaration: a token contract (maybe unresolved) or the chain asset. */ +interface TssAssetSource { + label: string; + contract?: string; + native?: boolean; + /** Token standard, only when statics classifies the declared token. */ + standard?: 'erc20' | 'specialized'; +} + +/** + * Collect every locally trusted asset declaration for an intent: recipient `tokenAddress` / + * `tokenName` / `tokenData`, the declared top-level token name, and the coin instance itself. + * Nothing read from the transaction contributes, so a server cannot make its own token selection + * look like the user's. + */ +function collectTssAssetSources( + chain: string, + declaredTokenName: string | undefined, + recipients: TssExpectedRecipient[] +): TssAssetSource[] { + const sources: TssAssetSource[] = []; + const chainCoin = coins.getOrUndefined(chain); + const addTokenNameSource = (tokenName: string): void => { + const staticsCoin = coins.getOrUndefined(tokenName); + if (!staticsCoin) { + sources.push({ label: `token '${tokenName}'` }); + return; + } + // A declaration only speaks for an asset on this chain. `coins` also resolves names, ids and + // aliases from other networks (a non-EVM coin, a token of another chain), and such a name says + // nothing about what this transaction moves: it stays an unresolved declaration so the caller + // is warned, never treated as this chain's native asset or as a local token contract. + if (!chainCoin || staticsCoin.network !== chainCoin.network) { + sources.push({ label: `asset '${tokenName}' on another network` }); + return; + } + // The name of this chain's native coin is a declaration of the chain asset, not of a token. + if (!(staticsCoin instanceof ContractAddressDefinedToken) || !staticsCoin.isToken) { + sources.push({ label: `coin '${tokenName}'`, native: true }); + return; + } + const contract = String(staticsCoin.contractAddress); + // ERC-20 expectations come from the statics class family, never from "has a contract address": + // NFT classes stay unclassified and ERC-7984 is specialised. + const standard = classifyTssTokenStandard(staticsCoin); + sources.push( + EVM_ADDRESS_PATTERN.test(contract) + ? { label: `token '${tokenName}'`, contract, standard } + : { label: `token '${tokenName}'`, standard } + ); + }; + const addContractSource = (contract: string, label: string): void => { + sources.push(EVM_ADDRESS_PATTERN.test(contract) ? { label, contract } : { label }); + }; + recipients.forEach((recipient) => { + if (recipient.tokenAddress !== undefined) { + addContractSource(recipient.tokenAddress, `recipient tokenAddress '${recipient.tokenAddress}'`); + } + if (recipient.tokenName !== undefined) { + addTokenNameSource(recipient.tokenName); + } + if (recipient.tokenData?.tokenContractAddress !== undefined) { + addContractSource( + recipient.tokenData.tokenContractAddress, + `recipient tokenData tokenContractAddress '${recipient.tokenData.tokenContractAddress}'` + ); + } + if (recipient.tokenData?.tokenName !== undefined) { + addTokenNameSource(recipient.tokenData.tokenName); + } + }); + if (declaredTokenName) { + addTokenNameSource(declaredTokenName); + } + if ( + chainCoin instanceof ContractAddressDefinedToken && + chainCoin.isToken && + EVM_ADDRESS_PATTERN.test(String(chainCoin.contractAddress)) + ) { + sources.push({ + label: `the coin token '${chain}'`, + contract: String(chainCoin.contractAddress), + standard: classifyTssTokenStandard(chainCoin), + }); + } + return sources; +} + +/** + * Resolve the asset a payment is declared to move, from locally trusted identity only: the + * recipient's own `tokenAddress` / `tokenName` / `tokenData` declarations, the declared top-level + * token name, and the coin instance itself. Nothing read from the transaction contributes, so a + * server cannot make its own token selection look like the user's. + * + * No token identity at all means native. That is the SDK's own convention, not a guess: `wallet.send` + * / `sendMany` attach `tokenName` to a recipient only when the caller names a token, and a token + * transfer on an EVM chain is either a token coin (`tbsc:usd1`) or a `transferToken` intent. A + * payment on a native coin that names no token therefore declares a native send, and token calldata + * on it is a definite mismatch rather than an unknown asset. + * + * Identity that cannot be resolved to an EVM contract, or that names two different assets, is + * `unresolved`: the intended asset is unknown, so only the checks that do not depend on it may run. + */ +function resolveTssPaymentAsset( + chain: string, + declaredTokenName: string | undefined, + recipients: TssExpectedRecipient[] +): TssPaymentAsset { + const sources = collectTssAssetSources(chain, declaredTokenName, recipients); + const tokenSources = sources.filter((source) => !source.native); + if (!tokenSources.length) { + return { kind: 'native' }; + } + const unresolved = tokenSources.find((source) => source.contract === undefined); + if (unresolved) { + return { kind: 'unresolved', reason: `${unresolved.label} cannot be resolved to an EVM token contract` }; + } + const nativeSource = sources.find((source) => source.native); + if (nativeSource) { + const tokenSource = tokenSources[0] as { label: string }; + return { kind: 'unresolved', reason: `${nativeSource.label} and ${tokenSource.label} name different assets` }; + } + const [first, ...rest] = tokenSources as Array<{ label: string; contract: string }>; + const conflict = rest.find((source) => normalizeTssAddress(source.contract) !== normalizeTssAddress(first.contract)); + if (conflict) { + return { kind: 'unresolved', reason: `${first.label} and ${conflict.label} name different token contracts` }; + } + return { kind: 'token', contract: first.contract }; +} + +/** + * Flatten a stored tx request intent into the recipient shape the asset resolver reads, keeping the + * full `tokenData` (contract address included) that `tssRecipientsFromStoredIntent` drops. + */ +function tssExpectedRecipientsFromStoredIntent(intent: PopulatedIntent): TssExpectedRecipient[] { + return (intent.recipients ?? []).map((recipient) => ({ + address: recipient.address.address, + amount: String(recipient.amount.value), + ...(recipient.data !== undefined && { data: recipient.data }), + ...(recipient.tokenData?.tokenName !== undefined && { tokenName: recipient.tokenData.tokenName }), + ...(recipient.tokenData?.tokenContractAddress !== undefined && { + tokenData: { + ...(recipient.tokenData.tokenName !== undefined && { tokenName: recipient.tokenData.tokenName }), + tokenContractAddress: recipient.tokenData.tokenContractAddress, + ...(recipient.tokenData.tokenType !== undefined && { tokenType: recipient.tokenData.tokenType }), + }, + }), + })); +} + +/** + * The asset a consolidation intent declares. Unlike a payment, a consolidation has no "no token + * means native" convention: a generic `consolidate` that declares no asset at all may be a native + * send, an ERC-20 sweep or a specialised format, so it stays `undetermined` and only the checks its + * bytes establish are enforced. `native-declared` requires an authorised field that names the chain + * asset; a `consolidateToken` intent always declares a token, even when it names no contract. + */ +type TssConsolidationAsset = + | { kind: 'native-declared' } + | { kind: 'undetermined' } + | { kind: 'token'; contract: string; standard: 'erc20' | 'specialized' | 'unknown' } + | { kind: 'unresolved'; reason: string }; + +function resolveTssConsolidationAsset( + chain: string, + intentType: string, + declaredTokenName: string | undefined, + recipients: TssExpectedRecipient[] +): TssConsolidationAsset { + const sources = collectTssAssetSources(chain, declaredTokenName, recipients); + const tokenSources = sources.filter((source) => !source.native); + if (!tokenSources.length) { + if (intentType === 'consolidateToken') { + return { kind: 'unresolved', reason: 'the consolidateToken intent declares no token contract' }; + } + return sources.some((source) => source.native) ? { kind: 'native-declared' } : { kind: 'undetermined' }; + } + const unresolved = tokenSources.find((source) => source.contract === undefined); + if (unresolved) { + return { kind: 'unresolved', reason: `${unresolved.label} cannot be resolved to an EVM token contract` }; + } + const nativeSource = sources.find((source) => source.native); + if (nativeSource) { + return { + kind: 'unresolved', + reason: `${nativeSource.label} and ${tokenSources[0].label} name different assets`, + }; + } + const [first, ...rest] = tokenSources as Array; + const conflict = rest.find((source) => normalizeTssAddress(source.contract) !== normalizeTssAddress(first.contract)); + if (conflict) { + return { kind: 'unresolved', reason: `${first.label} and ${conflict.label} name different token contracts` }; + } + // Only a declaration statics can classify establishes the standard. An address-only declaration + // still fixes the contract, but says nothing about which calls it supports. + const standards = tokenSources + .map((source) => source.standard) + .filter((standard): standard is 'erc20' | 'specialized' => standard !== undefined); + const standard: 'erc20' | 'specialized' | 'unknown' = + standards.length > 0 && standards.every((value) => value === standards[0]) ? standards[0] : 'unknown'; + return { kind: 'token', contract: first.contract, standard }; +} + export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implements CoinWithSignableConsistency { static hopTransactionSalt = 'bitgoHopAddressRequestSalt'; protected readonly sendMethodName: 'sendMultiSig' | 'sendMultiSigToken'; @@ -3382,10 +3965,23 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem * /** @inheritdoc CoinWithSignableConsistency */ assertSignableConsistency(serializedTxHex: string, signableHex: string): void { - const common = AbstractEthLikeNewCoins.getCustomChainCommon(this.getChainId()); - // getCustomChainCommon defaults to the london hardfork, which is only valid for chains - // that activated EIP-1559. Chains without it (e.g. BSC, XDC) sign under petersburg. - if (!this.staticsCoin?.features.includes(CoinFeature.EIP1559)) { + const network = this.getNetwork(); + if (!network) { + throw new InvalidTransactionError('cannot verify signable consistency without a coin network'); + } + // Build the common from this coin's own network: the bytes may belong to a token coin whose + // statics entry carries none of the base chain's fork features. + const common = getCommon(network); + // The fork is fixed before parsing, because the factory copies this Common when it decides + // whether the envelope is even valid on the chain. Chains without EIP-1559 (e.g. BSC, XDC, + // Kava) sign under petersburg, so a type-2 payload on them is rejected at parse exactly as it + // was before. An EIP-1559 chain keeps london, which parses both a type-0 legacy envelope and a + // type-2 fee-market envelope; the preimage below follows the envelope, not the fork. + const baseCoin = coins.getOrUndefined(this.getFamily()); + const chainSupportsEip1559 = baseCoin + ? baseCoin.features.includes(CoinFeature.EIP1559) + : this.staticsCoin?.features.includes(CoinFeature.EIP1559) ?? false; + if (!chainSupportsEip1559) { common.setHardfork(optionalDeps.EthCommon.Hardfork.Petersburg); } const ethTx = TransactionFactory.fromSerializedData(toBuffer(addHexPrefix(serializedTxHex)), { common }); @@ -3549,6 +4145,646 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem } } + /** + * Emit a precise, non-fatal warning for a check this verifier could not perform: identity that + * cannot be resolved locally, a format it does not parse, or an intent type with no handler. + * Never throws: a definite mismatch goes through the caller's mismatch thrower instead. + * + * The warning carries the intent type and the missing check only. Expected and decoded + * transaction fields are deliberately not logged: console output is shipped to log pipelines, + * and addresses / calldata must not be retained there just because a check was incomplete. + */ + protected emitTssVerificationWarning(reason: string, intentType: string): void { + console.warn(`[tss verification] ${intentType}: ${reason}`); + } + + /** + * Decode the bytes about to be signed. Only intent types this verifier understands reach this + * method, so special formats (FLR atomic exports, Tempo, VET, …) are never routed here. + */ + private async decodeTssVerificationTransaction(txHex?: string): Promise { + if (!txHex) { + throw new Error('missing txHex in txPrebuild'); + } + const txBuilder = this.getTransactionBuilder(); + txBuilder.from(txHex); + const txJson = (await txBuilder.build()).toJson(); + return { to: txJson.to ?? '', value: txJson.value ?? '0', data: txJson.data ?? '0x' }; + } + + /** + * Verify a payment against the bytes about to be signed: one recipient must match a single + * payment shape, several must match a batcher batch. + */ + private async verifyTssPaymentTransaction(args: { + intentType: string; + recipients: TssExpectedRecipient[]; + declaredTokenName?: string; + tx: DecodedTssTransaction; + throwRecipientMismatch: TssMismatchThrower; + reportUnverifiable: TssUnverifiableReporter; + }): Promise { + if (args.recipients.length > 1) { + await this.verifyTssBatchPayment(args); + return; + } + await this.verifyTssSinglePayment({ ...args, recipient: args.recipients[0] }); + } + + /** + * Verify one payment recipient. The native-send and ERC-20 `transfer` amount/destination + * comparisons are the checks this verifier always performed; the rest close the calldata shapes + * those checks left open and pin the asset identity the caller declared. + */ + private async verifyTssSinglePayment(args: { + intentType: string; + recipient: TssExpectedRecipient; + declaredTokenName?: string; + tx: DecodedTssTransaction; + throwRecipientMismatch: TssMismatchThrower; + reportUnverifiable: TssUnverifiableReporter; + }): Promise { + const { recipient, tx, intentType, throwRecipientMismatch, reportUnverifiable } = args; + const calldata = normalizeTssCalldata(tx.data); + // `''` / `'0x'` declare no calldata at all, exactly like omitting the field: the prebuild flow + // passes the caller's recipients through unchanged, and an empty string must not reclassify a + // native or ERC-20 payment as an exact arbitrary-calldata call. + const declaredCalldata = isTssEmptyCalldata(recipient.data) ? undefined : normalizeTssCalldata(recipient.data); + + // Historic checks: same comparisons and messages, now for every payment spelling. + if (isTssEmptyCalldata(calldata)) { + if (!tssValuesEqual(tx.value, recipient.amount)) { + await throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [ + { address: tx.to, amount: tx.value }, + ]); + } + if (normalizeTssAddress(tx.to) !== normalizeTssAddress(recipient.address)) { + await throwRecipientMismatch('destination address does not match with the recipient address', [ + { address: tx.to, amount: tx.value }, + ]); + } + } else if (calldata.startsWith(ERC20_TRANSFER_METHOD_ID)) { + // Malformed supported bytes throw here: the bytes cannot be verified against anything. + const declaredData = declaredCalldata ?? '0x'; + let expectedRecipient = normalizeTssAddress(recipient.address); + let expectedAmount = recipient.amount; + if (declaredData.startsWith(ERC20_TRANSFER_METHOD_ID)) { + // WalletConnect: the recipient declares its own transfer calldata instead of to/amount. + const [declaredRecipient, declaredAmount] = getRawDecoded( + ['address', 'uint256'], + tssCalldataPayload(declaredData) + ); + expectedRecipient = tssDecodedAddress(declaredRecipient); + expectedAmount = tssBigIntFromBuffer(declaredAmount).toString(); + } + const [innerRecipient, innerAmount] = getRawDecoded(['address', 'uint256'], tssCalldataPayload(calldata)); + const decodedRecipient = tssDecodedAddress(innerRecipient); + const decodedAmount = tssBigIntFromBuffer(innerAmount).toString(); + if (expectedAmount !== decodedAmount) { + await throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [ + { address: decodedRecipient, amount: decodedAmount }, + ]); + } + if (expectedRecipient !== decodedRecipient) { + await throwRecipientMismatch('destination address does not match with the recipient address', [ + { address: decodedRecipient, amount: decodedAmount }, + ]); + } + } + + // A recipient that declares raw calldata is a WalletConnect / dApp call: the caller authorised + // exactly these bytes, so `to`, `value` (a nonzero native value included) and `data` must all + // match. This is the only payment shape that may carry arbitrary calldata. + if (declaredCalldata !== undefined) { + if (normalizeTssAddress(tx.to) !== normalizeTssAddress(recipient.address)) { + await throwRecipientMismatch('destination address does not match with the recipient address', [ + { address: tx.to, amount: tx.value }, + ]); + } + if (!tssValuesEqual(tx.value, recipient.amount)) { + await throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [ + { address: tx.to, amount: tx.value }, + ]); + } + if (calldata !== declaredCalldata) { + await throwRecipientMismatch('transaction calldata does not match the calldata given by the client', [ + { address: tx.to, amount: tx.value }, + ]); + } + return; + } + + // NFT payments move calldata this verifier does not model; report instead of guessing. Only the + // NFT token types take this path: an ordinary ERC-20 recipient also carries tokenData, and its + // contract / native value / calldata shape are verified below. + if (isNftTokenType(recipient.tokenData?.tokenType)) { + reportUnverifiable( + intentType, + `payment declares tokenData.tokenType '${recipient.tokenData?.tokenType}': NFT payment verification is ` + + 'not implemented, transaction bytes were not compared against the declared token' + ); + return; + } + + const asset = resolveTssPaymentAsset(this.getChain(), args.declaredTokenName, [recipient]); + const selector = calldata.slice(0, 10); + const batchIds = tssBatchMethodIds(); + const isConfidential = + selector === confidentialTransferWithProofMethodId || selector === confidentialTransferNoProofMethodId; + const isBatch = selector === batchIds.native || selector === batchIds.token; + if (asset.kind === 'unresolved') { + reportUnverifiable(intentType, `payment ${asset.reason}: the intended asset cannot be verified`); + } + + if (asset.kind === 'native') { + if (isTssEmptyCalldata(calldata)) { + return; // to/value were compared above. + } + if (selector === ERC20_TRANSFER_METHOD_ID || isConfidential || isBatch) { + await throwRecipientMismatch( + 'payment intent declares a native asset but the transaction carries token or batch calldata', + [{ address: tx.to, amount: tx.value }] + ); + } + await throwRecipientMismatch( + 'payment intent calldata is not a recognized payment shape: a native payment must not carry calldata', + [{ address: tx.to, amount: tx.value }] + ); + return; + } + + if (asset.kind === 'token') { + if (!tssValuesEqual(tx.value, '0')) { + await throwRecipientMismatch('token transfer must not carry native value', [ + { address: tx.to, amount: tx.value }, + ]); + } + if (normalizeTssAddress(tx.to) !== normalizeTssAddress(asset.contract)) { + await throwRecipientMismatch('transaction to is not the intent token contract', [ + { address: tx.to, amount: tx.value }, + ]); + } + if (isTssEmptyCalldata(calldata)) { + await throwRecipientMismatch( + 'token payment requires ERC-20 transfer(address,uint256) or ERC-7984 confidentialTransfer calldata; ' + + 'the transaction data payload is empty', + [{ address: tx.to, amount: tx.value }] + ); + } + if (selector === ERC20_TRANSFER_METHOD_ID) { + // The historic check above already compared the inner recipient and amount. + assertCanonicalStaticTssArgs(calldata, 2, 'ERC-20 transfer'); + return; + } + if (isConfidential) { + await this.verifyTssConfidentialPayment({ + calldata, + recipient, + intentType, + throwRecipientMismatch, + reportUnverifiable, + }); + return; + } + if (isBatch) { + await throwRecipientMismatch('batch calldata cannot be verified against a single declared recipient', [ + { address: tx.to, amount: tx.value }, + ]); + } + await throwRecipientMismatch( + 'payment intent calldata is not a recognized payment shape (native send, ERC-20 transfer, ERC-7984 ' + + 'confidentialTransfer, or batch): refusing to treat it as a payment', + [{ address: tx.to, amount: tx.value }] + ); + return; + } + + // Asset unresolved: enforce every check that does not depend on knowing the asset. + if (isBatch) { + await throwRecipientMismatch('batch calldata cannot be verified against a single declared recipient', [ + { address: tx.to, amount: tx.value }, + ]); + } + if (selector === ERC20_TRANSFER_METHOD_ID) { + assertCanonicalStaticTssArgs(calldata, 2, 'ERC-20 transfer'); + return; // inner recipient/amount were compared above + } + if (isConfidential) { + await this.verifyTssConfidentialPayment({ + calldata, + recipient, + intentType, + throwRecipientMismatch, + reportUnverifiable, + }); + return; + } + await throwRecipientMismatch( + 'payment intent calldata is not a recognized payment shape (native send, ERC-20 transfer, ERC-7984 ' + + 'confidentialTransfer, or batch): refusing to treat it as a payment', + [{ address: tx.to, amount: tx.value }] + ); + } + + /** + * Verify an ERC-7984 `confidentialTransfer` payment: the selector's own recipient must match; + * the amount is encrypted, so it cannot be compared and is reported as unverified. + */ + private async verifyTssConfidentialPayment(args: { + calldata: string; + recipient: TssExpectedRecipient; + intentType: string; + throwRecipientMismatch: TssMismatchThrower; + reportUnverifiable: TssUnverifiableReporter; + }): Promise { + const { calldata, recipient, intentType } = args; + const withProof = calldata.startsWith(confidentialTransferWithProofMethodId); + if (withProof) { + assertCanonicalBytesTailTssArgs(calldata, 2, 'confidentialTransfer'); + } else { + assertCanonicalStaticTssArgs(calldata, 2, 'confidentialTransfer'); + } + const [innerRecipient] = getRawDecoded( + withProof ? confidentialTransferWithProofTypes : confidentialTransferNoProofTypes, + tssCalldataPayload(calldata) + ); + const decodedRecipient = tssDecodedAddress(innerRecipient); + if (decodedRecipient !== normalizeTssAddress(recipient.address)) { + await args.throwRecipientMismatch( + 'confidentialTransfer recipient does not match the recipient address given by the client', + [{ address: decodedRecipient, amount: '' }] + ); + } + args.reportUnverifiable( + intentType, + 'confidentialTransfer carries an encrypted amount: the amount cannot be compared with the declared amount' + ); + } + + /** + * Verify a multi-recipient payment: `to` is the network batcher and the decoded arrays must + * match the intent 1:1. Native batches use `batch(address[],uint256[])` with `value` equal to + * the sum; token batches use `batchTransferFrom(address,address[],uint256[])` with `value` 0 and + * the intent token contract. + */ + private async verifyTssBatchPayment(args: { + intentType: string; + recipients: TssExpectedRecipient[]; + declaredTokenName?: string; + tx: DecodedTssTransaction; + throwRecipientMismatch: TssMismatchThrower; + reportUnverifiable: TssUnverifiableReporter; + }): Promise { + const { recipients, tx, intentType, throwRecipientMismatch, reportUnverifiable } = args; + const calldata = normalizeTssCalldata(tx.data); + const totalAmount = recipients.reduce((sum, recipient) => sum + BigInt(recipient.amount), 0n).toString(); + const batcher = this.getNetwork()?.batcherContractAddress; + if (!batcher) { + reportUnverifiable( + intentType, + 'no batcher contract address is configured for this network: the batch payment destination cannot be verified' + ); + } else if (normalizeTssAddress(tx.to) !== normalizeTssAddress(batcher)) { + await throwRecipientMismatch('batch transaction to does not match the network batcher contract', [ + { address: tx.to, amount: tx.value }, + ]); + } + + // Each recipient resolves its own asset. Unresolved identities are reported and every other + // (definite) batch comparison still runs below. + const assets = recipients.map((recipient, index) => { + const asset = resolveTssPaymentAsset(this.getChain(), args.declaredTokenName, [recipient]); + if (asset.kind === 'unresolved') { + reportUnverifiable(intentType, `batch payment recipient ${index}: ${asset.reason}`); + } + return asset; + }); + const resolvedAssets = assets.filter( + (asset): asset is Exclude => asset.kind !== 'unresolved' + ); + let batchAsset: TssPaymentAsset = resolvedAssets[0] ?? { + kind: 'unresolved', + reason: 'no batch recipient declares a resolvable asset', + }; + const mixedAssets = resolvedAssets.some( + (asset) => + asset.kind !== batchAsset.kind || + (asset.kind === 'token' && + batchAsset.kind === 'token' && + normalizeTssAddress(asset.contract) !== normalizeTssAddress(batchAsset.contract)) + ); + if (mixedAssets) { + reportUnverifiable( + intentType, + 'batch payment recipients declare more than one asset: one batch call cannot be verified' + ); + batchAsset = { kind: 'unresolved', reason: 'batch recipients declare more than one asset' }; + } + + const compareRecipients = async ( + batchRecipients: Array<{ address: string; amount: string }>, + expectedValue: string, + valueMessage: string + ): Promise => { + if (batchRecipients.length !== recipients.length) { + await throwRecipientMismatch( + `batch calldata recipient count (${batchRecipients.length}) does not match intent recipients (${recipients.length})`, + [{ address: tx.to, amount: tx.value }] + ); + } + for (let index = 0; index < batchRecipients.length; index++) { + const want = recipients[index]; + const got = batchRecipients[index]; + if ( + normalizeTssAddress(got.address) !== normalizeTssAddress(want.address) || + !tssValuesEqual(got.amount, want.amount) + ) { + await throwRecipientMismatch(`batch calldata entry ${index} does not match intent recipient`, [ + { address: got.address, amount: got.amount }, + ]); + } + } + if (!tssValuesEqual(tx.value, expectedValue)) { + await throwRecipientMismatch(valueMessage, [{ address: tx.to, amount: tx.value }]); + } + }; + + const batchIds = tssBatchMethodIds(); + const selector = calldata.slice(0, 10); + if (selector === batchIds.native) { + if (batchAsset.kind === 'token') { + await throwRecipientMismatch( + 'token batch payment must use batchTransferFrom(address,address[],uint256[]) calldata', + [{ address: tx.to, amount: tx.value }] + ); + } + const batch = decodeTssBatchCalldata(calldata, false); + await compareRecipients( + batch.recipients, + totalAmount, + 'batch transaction value does not match the sum of intent amounts' + ); + return; + } + if (selector === batchIds.token) { + if (batchAsset.kind === 'native') { + await throwRecipientMismatch('native batch payment must use batch(address[],uint256[]) calldata', [ + { address: tx.to, amount: tx.value }, + ]); + } + const batch = decodeTssBatchCalldata(calldata, true); + if (batchAsset.kind === 'token') { + if (normalizeTssAddress(batch.tokenContract) !== normalizeTssAddress(batchAsset.contract)) { + await throwRecipientMismatch('batchTransferFrom token contract does not match the intent token contract', [ + { address: batch.tokenContract ?? '', amount: tx.value }, + ]); + } + } + await compareRecipients(batch.recipients, '0', 'token batch transaction must not carry native value'); + return; + } + await throwRecipientMismatch( + 'batch payment must use batch(address[],uint256[]) or batchTransferFrom(address,address[],uint256[]) calldata', + [{ address: tx.to, amount: tx.value }] + ); + } + + /** + * Validate the envelope of an intent type this verifier has no intent rules for. Malformed bytes + * must never reach signing just because the intent is unhandled, so the bytes still have to + * decode as an EVM transaction. Formats that are not EVM are never recognised here: a coin that + * owns such a format (e.g. FLR atomic transactions) must handle it in its own verifier override + * before delegating to this one. + */ + private async assertTssUnimplementedEncoding(txHex?: string): Promise { + if (!txHex) { + throw new Error('missing txHex in txPrebuild'); + } + try { + await this.decodeTssVerificationTransaction(txHex); + } catch (e) { + throw new Error(`transaction bytes could not be decoded as an EVM transaction: ${(e as Error).message}`); + } + } + + /** + * Verify the shape of a consolidation recognised from the intent type, a consolidate id, or the + * caller's `verification.consolidationToBaseAddress` flag. Only two consolidation shapes are + * established by this SDK: + * + * - a native sweep is a plain send to the wallet base address and carries no calldata; + * - a token sweep is exactly `transfer(baseAddress, amount)` on the declared ERC-20 contract with + * no native value. + * + * Only established expectations reject; everything else is reported, never claimed: + * + * - an authorised field that names the chain asset (`native-declared`) makes any calldata a + * definite mismatch, because a native sweep is a plain send; + * - a declared token sweep never carries native value, whatever calldata shape it uses; + * - a token contract statics classifies as ERC-20 must be swept with `transfer(address,uint256)`: + * any other function, batch encodings included, is a definite mismatch; + * - the flag-driven path keeps its historic rule that any calldata which is not an ERC-20 + * transfer must pay the wallet base address with `tx.to`. + * + * A generic `consolidate` that declares no asset at all is deliberately *not* treated as native: + * its bytes may belong to an ERC-20 or specialised sweep (the v6 server builds token sweeps on + * the plain `consolidate` type), so a non-empty shape is reported as unverifiable. A specialised + * token contract (ERC-7984) is verified by its own coin and is never rejected here. + * + * `tx.to` is the token contract for the token shape, so it is compared with the token identity the + * intent declares (caller declarations first, otherwise the stored intent's), resolved through + * the coin's own statics: a caller or stored `tokenName`, a recipient `tokenContractAddress` / + * `tokenAddress`, or the coin instance itself. A missing contract never suppresses the checks + * that do not need it. + * + * No amount rule applies: a sweep moves whatever the address holds, so the calldata amount is + * never compared with an intent amount. + */ + private async verifyTssConsolidationShape(args: { + intentType: string; + tx: DecodedTssTransaction; + baseAddress: string; + /** The caller set `verification.consolidationToBaseAddress`: its historic rejections apply. */ + flagDriven: boolean; + declaredTokenName?: string; + declaredRecipients: TssExpectedRecipient[]; + throwRecipientMismatch: TssMismatchThrower; + reportUnverifiable: TssUnverifiableReporter; + }): Promise { + const { tx, baseAddress, intentType, throwRecipientMismatch, reportUnverifiable } = args; + const calldata = normalizeTssCalldata(tx.data); + const asset = resolveTssConsolidationAsset( + this.getChain(), + intentType, + args.declaredTokenName, + args.declaredRecipients + ); + const declaresToken = asset.kind === 'token' || asset.kind === 'unresolved'; + const isErc20Transfer = calldata.startsWith(ERC20_TRANSFER_METHOD_ID); + const batchIds = tssBatchMethodIds(); + const isBatch = calldata.startsWith(batchIds.native) || calldata.startsWith(batchIds.token); + // Only a token statics classifies as ERC-20 has a single known sweep encoding. + const hasKnownErc20Contract = asset.kind === 'token' && asset.standard === 'erc20'; + const tokenContractCannotBeChecked = `consolidation ${ + asset.kind === 'unresolved' ? asset.reason : 'declares no token contract' + }: the token contract cannot be checked, only the wallet base address and native value were verified`; + const rejectNativeValue = (): Promise => + throwRecipientMismatch('Consolidation token transfer must not carry native value', [ + { address: tx.to, amount: tx.value }, + ]); + const rejectNativeCalldata = (): Promise => + throwRecipientMismatch(TSS_CONSOLIDATION_NATIVE_CALLDATA_MESSAGE, [{ address: tx.to, amount: tx.value }]); + + // No consolidation shape is a contract creation: both a native send and a token transfer need a + // destination, and an empty `to` can only deploy the calldata as code. + if (!tx.to) { + await throwRecipientMismatch('Consolidation transaction is missing recipient address', []); + } + + // Historic flag rejection, enforced before any warning: the flag has always required a tx that + // is not an ERC-20 transfer to pay the wallet base address with `tx.to`. Batch and arbitrary + // calldata are included, so nothing below may downgrade them to a report. + if (args.flagDriven && !isErc20Transfer && normalizeTssAddress(tx.to) !== normalizeTssAddress(baseAddress)) { + await throwRecipientMismatch('Consolidation transaction recipient does not match wallet base address', [ + { address: tx.to, amount: tx.value }, + ]); + } + + if (isErc20Transfer) { + // Canonical ERC-20 transfer(address,uint256) only: a trailing argument or a shorter payload + // would call something this verifier has not modelled. + assertCanonicalStaticTssArgs(calldata, 2, 'ERC-20 transfer'); + // For ERC-20 consolidations `tx.to` is the token contract; the recipient is in the calldata. + const [innerRecipient] = getRawDecoded(['address', 'uint256'], tssCalldataPayload(calldata)); + const decodedRecipient = tssDecodedAddress(innerRecipient); + if (decodedRecipient !== normalizeTssAddress(baseAddress)) { + await throwRecipientMismatch('Consolidation transaction recipient does not match wallet base address', [ + { address: decodedRecipient, amount: tx.value }, + ]); + } + if (!tssValuesEqual(tx.value, '0')) { + await rejectNativeValue(); + } + if (asset.kind === 'token') { + if (normalizeTssAddress(tx.to) !== normalizeTssAddress(asset.contract)) { + await throwRecipientMismatch('Consolidation transaction does not call the declared token contract', [ + { address: tx.to, amount: tx.value }, + ]); + } + // Only a plain ERC-20 is verified by this shape (recipient, value, canonical args and + // contract). A specialised token is owned by its own coin, and an unclassified standard + // (NFT, dynamically generated token) is not established as ERC-20: both are reported. + if (asset.standard !== 'erc20') { + reportUnverifiable( + intentType, + asset.standard === 'specialized' + ? 'specialised token consolidation is verified by the token coin and is not modelled here' + : 'consolidation declares a token whose standard is not established as ERC-20: the token movement ' + + 'cannot be verified' + ); + } + return; + } + // A field declared the chain asset, so a token transfer contradicts the intent. + if (asset.kind === 'native-declared') { + await rejectNativeCalldata(); + } + reportUnverifiable(intentType, tokenContractCannotBeChecked); + return; + } + + if (isBatch) { + // A declared token sweep never carries native value, whatever batch encoding is used. For an + // unresolved token that does not depend on which contract it is: the intent still declares a + // token, exactly as in the non-batch branch below. + if ((hasKnownErc20Contract || asset.kind === 'unresolved') && !tssValuesEqual(tx.value, '0')) { + await rejectNativeValue(); + } + // An explicitly declared native asset cannot be moved by batcher calldata at all. + if (asset.kind === 'native-declared') { + await rejectNativeCalldata(); + } + // A known ERC-20 token has exactly one sweep encoding; a batch cannot carry it to the base + // address. Anything less certain stays a reported, unimplemented shape: an undeclared asset + // may be a specialised sweep, an unresolved one names no contract to judge. + if (hasKnownErc20Contract) { + await throwRecipientMismatch(TSS_CONSOLIDATION_TOKEN_SHAPE_MESSAGE, [{ address: tx.to, amount: tx.value }]); + } + reportUnverifiable(intentType, 'batch consolidation calldata cannot be verified against the wallet base address'); + return; + } + + if (isTssEmptyCalldata(calldata)) { + // A sweep that declares a token cannot move it without `transfer` calldata. + if (declaresToken) { + await throwRecipientMismatch( + 'Consolidation transaction declares a token but carries no ERC-20 transfer(address,uint256) calldata', + [{ address: tx.to, amount: tx.value }] + ); + } + if (normalizeTssAddress(tx.to) !== normalizeTssAddress(baseAddress)) { + await throwRecipientMismatch('Consolidation transaction recipient does not match wallet base address', [ + { address: tx.to, amount: tx.value }, + ]); + } + return; + } + + // Any other calldata is not a consolidation shape. + if (asset.kind === 'native-declared') { + await rejectNativeCalldata(); + } + // A known ERC-20 contract has exactly one sweep encoding and never carries native value. + if (asset.kind === 'token' && hasKnownErc20Contract) { + if (!tssValuesEqual(tx.value, '0')) { + await rejectNativeValue(); + } + if (normalizeTssAddress(tx.to) !== normalizeTssAddress(asset.contract)) { + await throwRecipientMismatch('Consolidation transaction does not call the declared token contract', [ + { address: tx.to, amount: tx.value }, + ]); + } + await throwRecipientMismatch(TSS_CONSOLIDATION_TOKEN_SHAPE_MESSAGE, [{ address: tx.to, amount: tx.value }]); + } + if (asset.kind === 'token') { + // A specialised token may legitimately be swept through its own contract entry point (the + // ERC-7984 token coin verifies those), so only a plain ERC-20 fixes `tx.to` here. How a + // specialised or unclassified standard behaves with native value is not established, so it is + // reported, never forced through the ERC-20 expectation. + if (asset.standard !== 'specialized' && normalizeTssAddress(tx.to) !== normalizeTssAddress(asset.contract)) { + await throwRecipientMismatch('Consolidation transaction does not call the declared token contract', [ + { address: tx.to, amount: tx.value }, + ]); + } + reportUnverifiable( + intentType, + asset.standard === 'specialized' + ? 'specialised token consolidation is verified by the token coin and is not modelled here' + : `consolidation calldata is not ERC-20 transfer(address,uint256)${ + tssValuesEqual(tx.value, '0') ? '' : ' and carries native value' + } and the declared token standard is unknown: the token movement cannot be verified` + ); + return; + } + if (asset.kind === 'unresolved') { + // The contract is unknown, but a token sweep still carries no native value. + if (!tssValuesEqual(tx.value, '0')) { + await rejectNativeValue(); + } + reportUnverifiable( + intentType, + `consolidation ${asset.reason}: neither the token contract nor the calldata can be verified` + ); + return; + } + // No asset is declared: the intent is a generic consolidation whose bytes may belong to a token + // or specialised sweep, so nothing here is a definite mismatch. + reportUnverifiable( + intentType, + 'consolidation calldata is not a native send and the intent declares no asset: the bytes cannot be verified' + ); + } + /** * @param {VerifyEthTransactionOptions} params * @param {TransactionParams} params.txParams - params object passed to send @@ -3558,7 +4794,16 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem * @throws {TxIntentMismatchRecipientError} if transaction recipients don't match user intent */ async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const { txParams, txPrebuild, wallet } = params; + const { txParams, txPrebuild, wallet, intent, callerTxParams } = params; + + // Exactly one source speaks for the user intent: the caller's own params when it declared a + // type or recipients, otherwise the stored tx request intent (pending approval). The other + // source is dropped, so server data never selects the handler or fills in what the caller + // left out. `txParams` alone cannot answer this: with a stored intent present, its `type` + // and `recipients` have already been back-filled from that server data. + const callerParams = callerTxParams ?? (intent ? undefined : txParams); + const callerDeclaresIntent = !!(callerParams?.type || callerParams?.recipients?.length); + const storedIntent = intent && !callerDeclaresIntent ? intent : undefined; // Helper to throw TxIntentMismatchRecipientError with recipient details const throwRecipientMismatch = async (message: string, mismatchedRecipients: Recipient[]): Promise => { @@ -3573,11 +4818,18 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem ); }; + // A check this verifier cannot perform (unresolved metadata, a format it does not parse, an + // intent type without a handler) is reported here and never silently dropped. Definite + // mismatches go through throwRecipientMismatch instead. + const reportUnverifiable: TssUnverifiableReporter = (intentType, reason) => + this.emitTssVerificationWarning(reason, intentType); + if ( !txParams?.recipients && !( txParams.prebuildTx?.consolidateId || txPrebuild?.consolidateId || + storedIntent?.consolidateId || txParams.stakingRequestId || txParams.prebuildTx?.stakingRequestId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) @@ -3593,69 +4845,81 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem throw new Error('tx cannot be both a batch and hop transaction'); } - if (txParams.type && ['transfer'].includes(txParams.type)) { - if (txParams.recipients && txParams.recipients.length === 1) { - const recipients = txParams.recipients; - const expectedAmount = recipients[0].amount.toString(); - const expectedDestination = recipients[0].address; - - const txBuilder = this.getTransactionBuilder(); - txBuilder.from(txPrebuild.txHex); - const tx = await txBuilder.build(); - const txJson = tx.toJson(); - if (txJson.data === '0x') { - if (expectedAmount !== txJson.value) { - await throwRecipientMismatch( - 'the transaction amount in txPrebuild does not match the value given by client', - [{ address: txJson.to, amount: txJson.value }] - ); - } - if (expectedDestination.toLowerCase() !== txJson.to.toLowerCase()) { - await throwRecipientMismatch('destination address does not match with the recipient address', [ - { address: txJson.to, amount: txJson.value }, - ]); - } - } else if (txJson.data.startsWith('0xa9059cbb')) { - const [recipientAddress, amount] = getRawDecoded( - ['address', 'uint256'], - getBufferedByteCode('0xa9059cbb', txJson.data) - ); - - // Check if recipients[0].data exists (WalletConnect flow) - let expectedRecipientAddress: string; - let expectedTokenAmount: string; - const recipientData = (recipients[0] as any).data; - - if (recipientData && recipientData.startsWith('0xa9059cbb')) { - // WalletConnect: decode expected recipient and amount from recipients[0].data - const [expectedRecipient, expectedAmount] = getRawDecoded( - ['address', 'uint256'], - getBufferedByteCode('0xa9059cbb', recipientData) - ); - expectedRecipientAddress = addHexPrefix(expectedRecipient.toString()).toLowerCase(); - expectedTokenAmount = expectedAmount.toString(); - } else { - // Normal flow: use recipients[0].address and recipients[0].amount - expectedRecipientAddress = expectedDestination.toLowerCase(); - expectedTokenAmount = expectedAmount; - } - - if (expectedTokenAmount !== amount.toString()) { - await throwRecipientMismatch( - 'the transaction amount in txPrebuild does not match the value given by client', - [{ address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() }] - ); - } + // Canonical verifier intent type, derived only from the source that speaks for the user: the + // stored intent type when it was selected, otherwise the caller's own declared type. It is + // never inferred from calldata, and `payment` is the stored spelling of `transfer`. + const explicitType = storedIntent + ? canonicalTssIntentType(storedIntent.intentType) + : canonicalTssIntentType(callerParams?.type); + // A consolidation id on the prebuild or the stored intent is server data. It marks the tx as + // a consolidation only when the caller declared nothing of its own; it can never reroute a + // caller-declared payment to the consolidate checks. + const consolidateId = callerDeclaresIntent + ? undefined + : txParams.prebuildTx?.consolidateId || txPrebuild?.consolidateId || storedIntent?.consolidateId || undefined; + const isConsolidation = + !!params.verification?.consolidationToBaseAddress || + explicitType === 'consolidate' || + explicitType === 'consolidateToken' || + !!consolidateId; + const canonicalType = explicitType || (isConsolidation ? 'consolidate' : 'transfer'); + + // contractCall / customTx carry arbitrary calldata with no intent shape to verify — never let + // them reach signing on this path. + if (EVM_TSS_UNSUPPORTED_INTENT_TYPES.has(canonicalType)) { + throw new Error(`unsupported intent type for EVM TSS: ${canonicalType}`); + } + + // A caller that names a payment (`transfer` / `payment`) must supply the recipients it is + // paying: with none there is nothing to verify the bytes against, and the stored intent was + // dropped (its destination may be attacker-chosen). `[]` declares nothing, like omitting it. + // No mode may downgrade an unverifiable payment to a warning, and a token transfer + // (`transferToken`) keeps its own specialised handling rather than inheriting this rule. + if (canonicalTssIntentType(callerParams?.type) === 'transfer' && !callerParams?.recipients?.length) { + await throwRecipientMismatch( + 'caller declared a payment without recipients: pass txParams.recipients so the transaction can be ' + + 'verified against the user intent', + [] + ); + } - if (expectedRecipientAddress !== addHexPrefix(recipientAddress.toString()).toLowerCase()) { - await throwRecipientMismatch('destination address does not match with the recipient address', [ - { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() }, - ]); - } - } + // Payments (`transfer` / `payment`) must match the closed set of payment shapes in + // verifyTssPaymentTransaction. Hop transactions keep their dedicated validateHopPrebuild path. + if (canonicalType === 'transfer' && !txParams.hop) { + const declaredRecipients = storedIntent + ? txParams.recipients?.length + ? txParams.recipients + : tssRecipientsFromStoredIntent(storedIntent) + : callerParams?.recipients ?? []; + const declaredTokenName = storedIntent ? txParams.tokenName ?? storedIntent.tokenName : callerParams?.tokenName; + if (!declaredRecipients.length) { + reportUnverifiable( + 'transfer', + 'payment intent has no recipients: the transaction bytes cannot be verified against intent' + ); + } else { + await this.verifyTssPaymentTransaction({ + intentType: 'transfer', + recipients: toTssExpectedRecipients(declaredRecipients, storedIntent), + declaredTokenName, + tx: await this.decodeTssVerificationTransaction(txPrebuild.txHex), + throwRecipientMismatch, + reportUnverifiable, + }); } } + // A stored DeFi leg carries only the server-supplied vault id and amount; no local registry + // anchors its target contract or approve spender, so it is reported rather than checked + // against bytes that only prove the server agrees with itself. + if (storedIntent && ['defiApprove', 'defiDeposit', 'defiWithdraw'].includes(canonicalType)) { + reportUnverifiable( + canonicalType, + `stored ${canonicalType} intent cannot be verified: the transaction target and approve spender ` + + 'cannot be anchored to a locally known vault contract' + ); + } + if (txParams.type && ['defiApprove', 'defiDeposit', 'defiWithdraw'].includes(txParams.type)) { if (!txPrebuild.txHex) { throw new Error('missing txHex in txPrebuild'); @@ -3752,52 +5016,72 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem await this.assertWrapperApproveIsDeclared(params, throwRecipientMismatch); } - // Verify consolidation transactions send to base address + // Consolidation verification: one shape verifier serves both the caller's + // `verification.consolidationToBaseAddress` flag and a consolidation recognised from the intent + // type (`consolidate`, `consolidateToken`) or a consolidate id. The flag keeps its historic + // requirement that the wallet base address is known; the recognised-intent path decodes the + // bytes first and reports a missing base address instead, so a malformed payload never becomes + // signable just because the base address is unavailable. + const coinSpecificBaseAddress = wallet.coinSpecific()?.baseAddress; + const walletBaseAddress = typeof coinSpecificBaseAddress === 'string' ? coinSpecificBaseAddress : undefined; + // Token identity is read only from the source that speaks for the user: the caller's own + // declarations when it declared the intent, otherwise the stored tx request intent. Never from + // the transaction bytes. + const consolidationTokenName = storedIntent + ? txParams.tokenName ?? storedIntent.tokenName + : callerParams?.tokenName; + const consolidationDeclaredRecipients: TssExpectedRecipient[] = storedIntent + ? tssExpectedRecipientsFromStoredIntent(storedIntent) + : toTssExpectedRecipients(callerParams?.recipients ?? []); + const consolidationIntentType = canonicalType === 'consolidateToken' ? 'consolidateToken' : 'consolidate'; if (params.verification?.consolidationToBaseAddress) { - const coinSpecific = wallet.coinSpecific(); - if (!coinSpecific || !coinSpecific.baseAddress) { + if (!walletBaseAddress) { throw new Error('Unable to determine base address for consolidation'); } - const baseAddress = coinSpecific.baseAddress; - - if (!txPrebuild.txHex) { - throw new Error('missing txHex in txPrebuild'); - } - - const txBuilder = this.getTransactionBuilder(); - txBuilder.from(txPrebuild.txHex); - const tx = await txBuilder.build(); - const txJson = tx.toJson(); - - // Verify the transaction recipient matches the base address - if (!txJson.to) { - throw new Error('Consolidation transaction is missing recipient address'); + await this.verifyTssConsolidationShape({ + intentType: consolidationIntentType, + tx: await this.decodeTssVerificationTransaction(txPrebuild.txHex), + baseAddress: walletBaseAddress, + flagDriven: true, + declaredTokenName: consolidationTokenName, + declaredRecipients: consolidationDeclaredRecipients, + throwRecipientMismatch, + reportUnverifiable, + }); + } else if (isConsolidation) { + const consolidationTx = await this.decodeTssVerificationTransaction(txPrebuild.txHex); + if (!walletBaseAddress) { + reportUnverifiable( + consolidationIntentType, + 'wallet base address is not available: the consolidation destination cannot be verified' + ); + } else { + await this.verifyTssConsolidationShape({ + intentType: consolidationIntentType, + tx: consolidationTx, + baseAddress: walletBaseAddress, + flagDriven: false, + declaredTokenName: consolidationTokenName, + declaredRecipients: consolidationDeclaredRecipients, + throwRecipientMismatch, + reportUnverifiable, + }); } + } - const erc20TransferSelector = addHexPrefix( - optionalDeps.ethAbi.methodID('transfer', ['address', 'uint256']).toString('hex') + // Every other intent type has no fail-closed verifier here. Say so precisely instead of + // implying the bytes were checked; specialized formats (FLR atomic exports, Tempo, VET, …) + // are never routed through the EVM parser above. + if ( + !['transfer', 'consolidate', 'consolidateToken'].includes(canonicalType) && + !EVM_TSS_DEDICATED_CHECK_INTENT_TYPES.has(canonicalType) + ) { + await this.assertTssUnimplementedEncoding(txPrebuild.txHex); + reportUnverifiable( + canonicalType || String(txParams.type ?? ''), + `no fail-closed verifier for intent type '${canonicalType || txParams.type}': the bytes are a valid EVM ` + + 'transaction but were not compared against intent' ); - - if (txJson.data && txJson.data.startsWith(erc20TransferSelector)) { - // For ERC20 token consolidations, txJson.to is the token contract address, - // not the actual transfer recipient. The real recipient is encoded in the - // transfer(address,uint256) calldata. Decode it and verify against baseAddress. - const [recipientAddress] = getRawDecoded( - ['address', 'uint256'], - getBufferedByteCode(erc20TransferSelector, txJson.data) - ); - const decodedRecipient = addHexPrefix(recipientAddress.toString()).toLowerCase(); - if (decodedRecipient !== baseAddress.toLowerCase()) { - await throwRecipientMismatch('Consolidation transaction recipient does not match wallet base address', [ - { address: decodedRecipient, amount: txJson.value }, - ]); - } - } else if (txJson.to.toLowerCase() !== baseAddress.toLowerCase()) { - // Native coin consolidation: txJson.to is the actual recipient - await throwRecipientMismatch('Consolidation transaction recipient does not match wallet base address', [ - { address: txJson.to, amount: txJson.value }, - ]); - } } return true; diff --git a/modules/abstract-eth/test/unit/index.ts b/modules/abstract-eth/test/unit/index.ts index bd8b8e53b93..6803cb99a55 100644 --- a/modules/abstract-eth/test/unit/index.ts +++ b/modules/abstract-eth/test/unit/index.ts @@ -5,3 +5,4 @@ export * from './coin'; export * from './messages'; export * from './zamaUtils'; export * from './decryptionDelegationBuilder'; +export * from './tssVerification'; diff --git a/modules/abstract-eth/test/unit/tssVerification.ts b/modules/abstract-eth/test/unit/tssVerification.ts new file mode 100644 index 00000000000..c5e4f4a52d8 --- /dev/null +++ b/modules/abstract-eth/test/unit/tssVerification.ts @@ -0,0 +1,1360 @@ +import 'should'; +import BN from 'bn.js'; +import EthereumCommon from '@ethereumjs/common'; +import { FeeMarketEIP1559Transaction } from '@ethereumjs/tx'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; +import { + BaseCoin as StaticsBaseCoin, + coins, + ContractAddressDefinedToken, + EthereumNetwork as EthLikeNetwork, +} from '@bitgo/statics'; +import { BitGoBase, TransactionType } from '@bitgo/sdk-core'; + +import { AbstractEthLikeNewCoins, Fee, optionalDeps, TransactionBuilder, TransferBuilder } from '../../src'; + +/** + * Focused coverage for the minimal EVM TSS payment + consolidation verification: + * definite mismatches throw, unverifiable checks warn, malformed bytes always fail. + */ +const chain = 'hteth'; +const staticsCoin = coins.get(chain); +const baseAddress = '0x1111111111111111111111111111111111111111'; +const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; +const attackerAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; +const tokenName = 'hteth:tusdc'; +const tokenContractAddress = String((coins.get(tokenName) as ContractAddressDefinedToken).contractAddress); +const batcherContractAddress = (staticsCoin.network as EthLikeNetwork).batcherContractAddress as string; + +class TestEthLikeCoin extends AbstractEthLikeNewCoins { + constructor(bitgo: BitGoBase, coinConfig: Readonly) { + super(bitgo, coinConfig); + } + + protected getTransactionBuilder(_common?: EthereumCommon): TransactionBuilder { + return new TestTransactionBuilder(this.staticsCoin as Readonly); + } +} + +/** + * Concrete builder for the raw payment / contract-call transactions this suite signs. The base + * `TransactionBuilder` is abstract only because a wallet `Send` needs a transfer encoding, which + * these fixtures never build. + */ +class TestTransactionBuilder extends TransactionBuilder { + transfer(): TransferBuilder { + return new TransferBuilder(); + } +} + +const word = (value: bigint): string => value.toString(16).padStart(64, '0'); +const addrWord = (address: string): string => address.toLowerCase().replace(/^0x/, '').padStart(64, '0'); + +function encodeErc20Transfer(to: string, amount: string): string { + return `${'0xa9059cbb'}${addrWord(to)}${word(BigInt(amount))}`; +} + +function encodeBatch(recipients: Array<{ address: string; amount: string }>): string { + const selector = optionalDeps.ethAbi.methodID('batch', ['address[]', 'uint256[]']); + const args = optionalDeps.ethAbi.rawEncode( + ['address[]', 'uint256[]'], + [recipients.map((recipient) => recipient.address), recipients.map((recipient) => new BN(recipient.amount))] + ); + return `0x${Buffer.concat([selector, args]).toString('hex')}`; +} + +function encodeBatchTransferFrom(tokenAddress: string, recipients: Array<{ address: string; amount: string }>): string { + const selector = optionalDeps.ethAbi.methodID('batchTransferFrom', ['address', 'address[]', 'uint256[]']); + const args = optionalDeps.ethAbi.rawEncode( + ['address', 'address[]', 'uint256[]'], + [tokenAddress, recipients.map((recipient) => recipient.address), recipients.map((r) => new BN(r.amount))] + ); + return `0x${Buffer.concat([selector, args]).toString('hex')}`; +} + +type Kernel = Parameters[0]; + +describe(`EVM TSS verification (${chain})`, function () { + let coin: TestEthLikeCoin; + let bitgo: TestBitGoAPI; + let warnings: string[]; + const originalWarn = console.warn; + + before(function () { + bitgo = TestBitGo.decorate(BitGoAPI, { env: 'test' }); + coin = new TestEthLikeCoin(bitgo, staticsCoin); + }); + + beforeEach(function () { + warnings = []; + console.warn = ((...args: unknown[]) => warnings.push(args.map(String).join(' '))) as typeof console.warn; + }); + + afterEach(function () { + console.warn = originalWarn; + }); + + async function buildTxFor( + coinConfig: Readonly, + opts: { to: string; value?: string; data?: string; eip1559?: boolean } + ): Promise { + const builder = new TestTransactionBuilder(coinConfig); + builder.type(opts.data && opts.data !== '0x' ? TransactionType.ContractCall : TransactionType.SingleSigSend); + builder.counter(1); + const fee: Fee = opts.eip1559 + ? { + fee: '10', + gasLimit: '100000', + eip1559: { maxFeePerGas: '10', maxPriorityFeePerGas: '1' }, + } + : { fee: '10', gasLimit: '100000' }; + builder.fee(fee); + builder.contract(opts.to); + if (opts.value !== undefined) { + builder.value(opts.value); + } + if (opts.data !== undefined) { + builder.data(opts.data); + } + return (await builder.build()).toBroadcastFormat(); + } + + const buildTx = (opts: { to: string; value?: string; data?: string }): Promise => + buildTxFor(staticsCoin, opts); + + function verify(params: { + caller?: Record; + intent?: Record; + txParams?: Record; + txHex: string; + omitBaseAddress?: boolean; + verification?: { consolidationToBaseAddress?: boolean }; + }): Promise { + return coin.verifyTssTransaction({ + callerTxParams: params.caller, + intent: params.intent, + txParams: params.txParams ?? params.caller ?? {}, + txPrebuild: { txHex: params.txHex }, + wallet: { coinSpecific: () => (params.omitBaseAddress ? {} : { baseAddress }) }, + verification: params.verification, + } as unknown as Kernel); + } + + describe('payments', function () { + it('verifies a native payment that matches the caller intent', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '1000' }); + const result = await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a native payment to a different destination', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/destination address does not match/); + }); + + it('rejects a native payment with a different value', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '999' }); + await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/does not match the value given by client/); + }); + + it('verifies the stored payment intent when the caller declared nothing', async function () { + const intent = { + intentType: 'payment', + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: chain } }], + }; + const txHex = await buildTx({ to: recipientAddress, value: '1000' }); + const result = await verify({ + intent, + txParams: { type: 'payment', recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects the stored payment intent when the bytes pay someone else', async function () { + const intent = { + intentType: 'payment', + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: chain } }], + }; + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await verify({ + intent, + txParams: { type: 'payment', recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/destination address does not match/); + }); + + it('rejects a caller-declared payment without recipients instead of using stored ones', async function () { + // The stored intent back-fills txParams.recipients, but the caller declared only the type: + // the stored recipients are server data and must not stand in for the caller intent. + const intent = { + intentType: 'payment', + recipients: [{ address: { address: attackerAddress }, amount: { value: '1000', symbol: chain } }], + }; + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await verify({ + caller: { type: 'transfer' }, + intent, + txParams: { type: 'payment', recipients: [{ address: attackerAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/caller declared a payment without recipients/); + }); + + it('verifies an ERC-20 payment to the declared token contract', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + const result = await verify({ + caller: { type: 'transfer', tokenName, recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects an ERC-20 payment to a different token contract', async function () { + const txHex = await buildTx({ + to: attackerAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + await verify({ + caller: { type: 'transfer', tokenName, recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/token contract/); + }); + + it('rejects an ERC-20 payment carrying native value', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '1', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + await verify({ + caller: { type: 'transfer', tokenName, recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/native value/); + }); + + it('rejects ERC-20 calldata on a payment that declares no token identity', async function () { + // No `tokenName` / `tokenAddress` / `tokenData` on a native-chain payment is a native send + // (the SDK convention for wallet.send / sendMany), so token calldata is a definite mismatch. + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/declares a native asset but the transaction carries token or batch calldata/); + }); + + it('verifies the same ERC-20 transfer once the recipient declares the token contract', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + const result = await verify({ + caller: { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '1000', tokenAddress: tokenContractAddress }], + }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects ERC-20 calldata with trailing bytes', async function () { + const data = `${encodeErc20Transfer(recipientAddress, '1000')}${'00'.repeat(32)}`; + const txHex = await buildTx({ to: tokenContractAddress, value: '0', data }); + await verify({ + caller: { type: 'transfer', tokenName, recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex, + }).should.be.rejectedWith(/not canonical/); + }); + + it('rejects an approve disguised as a native payment', async function () { + const data = `0x095ea7b3${addrWord(attackerAddress)}${word((1n << 256n) - 1n)}`; + const txHex = await buildTx({ to: recipientAddress, value: '0', data }); + await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '0' }] }, + txHex, + }).should.be.rejectedWith(/not a recognized payment shape/); + }); + + it('verifies an explicitly declared calldata payload exactly', async function () { + const data = `0x095ea7b3${addrWord(recipientAddress)}${word(1n)}`; + const txHex = await buildTx({ to: recipientAddress, value: '5', data }); + const result = await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '5', data }] }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects explicitly declared calldata that does not match', async function () { + const data = `0x095ea7b3${addrWord(recipientAddress)}${word(1n)}`; + const otherData = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: recipientAddress, value: '5', data }); + await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '5', data: otherData }] }, + txHex, + }).should.be.rejectedWith(/calldata does not match/); + }); + + it('warns about an unresolvable token name', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + const result = await verify({ + caller: { + type: 'transfer', + tokenName: 'hteth:not-a-registered-token', + recipients: [{ address: recipientAddress, amount: '1000' }], + }, + txHex, + }); + result.should.equal(true); + warnings.some((warning) => /cannot be resolved/.test(warning)).should.equal(true); + }); + + it('still rejects an inner amount mismatch when the token identity is unresolved', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '999'), + }); + await verify({ + caller: { + type: 'transfer', + tokenName: 'hteth:not-a-registered-token', + recipients: [{ address: recipientAddress, amount: '1000' }], + }, + txHex, + }).should.be.rejectedWith(/does not match the value given by client/); + }); + + it('treats empty declared calldata as no calldata for a native payment', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '1000' }); + const result = await verify({ + caller: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000', data: '' }] }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + describe('realistic ERC-20 tokenData', function () { + // The intent tokenData shape is used by ERC-20 too: tokenType 'ERC20' plus tokenQuantity and + // tokenName. It must not divert the payment to the NFT/incomplete path. + const erc20TokenData = () => ({ tokenType: 'ERC20', tokenQuantity: '1000', tokenName }); + + it('verifies an ERC-20 payment that carries ERC-20 tokenData', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + const result = await verify({ + caller: { + type: 'transfer', + tokenName, + recipients: [{ address: recipientAddress, amount: '1000', tokenData: erc20TokenData() }], + }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('does not reclassify an ERC-20 payment when the caller passes empty data', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + const result = await verify({ + caller: { + type: 'transfer', + tokenName, + recipients: [{ address: recipientAddress, amount: '1000', data: '', tokenData: erc20TokenData() }], + }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects an ERC-20 payment with ERC-20 tokenData sent to another contract', async function () { + const txHex = await buildTx({ + to: attackerAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + await verify({ + caller: { + type: 'transfer', + tokenName, + recipients: [{ address: recipientAddress, amount: '1000', tokenData: erc20TokenData() }], + }, + txHex, + }).should.be.rejectedWith(/token contract/); + }); + + it('rejects an ERC-20 payment with ERC-20 tokenData carrying native value', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '1', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + await verify({ + caller: { + type: 'transfer', + tokenName, + recipients: [{ address: recipientAddress, amount: '1000', tokenData: erc20TokenData() }], + }, + txHex, + }).should.be.rejectedWith(/native value/); + }); + + const storedIntent = () => ({ + intentType: 'payment', + tokenName, + recipients: [ + { + address: { address: recipientAddress }, + amount: { value: '1000', symbol: tokenName }, + tokenData: { + tokenType: 'ERC20', + tokenQuantity: '1000', + tokenName, + tokenContractAddress, + }, + }, + ], + }); + const effectiveParams = () => ({ + type: 'payment', + tokenName, + recipients: [{ address: recipientAddress, amount: '1000', tokenName }], + }); + + it('verifies a stored ERC-20 payment whose stored intent carries ERC-20 tokenData', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + const result = await verify({ + intent: storedIntent(), + txParams: effectiveParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a stored ERC-20 payment sent to another contract', async function () { + const txHex = await buildTx({ + to: attackerAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + await verify({ intent: storedIntent(), txParams: effectiveParams(), txHex }).should.be.rejectedWith( + /token contract/ + ); + }); + + it('rejects a stored ERC-20 payment carrying native value', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '1', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + await verify({ intent: storedIntent(), txParams: effectiveParams(), txHex }).should.be.rejectedWith( + /native value/ + ); + }); + }); + + it('warns about an NFT payment it cannot model', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '1000'), + }); + const result = await verify({ + caller: { + type: 'transfer', + tokenName, + recipients: [{ address: recipientAddress, amount: '1000', tokenData: { tokenType: 'ERC721' } }], + }, + txHex, + }); + result.should.equal(true); + warnings.some((warning) => /NFT payment verification is not implemented/.test(warning)).should.equal(true); + }); + }); + + describe('batch payments', function () { + const recipients = [ + { address: recipientAddress, amount: '1000' }, + { address: attackerAddress, amount: '2000' }, + ]; + + it('verifies a native batch to the network batcher', async function () { + const txHex = await buildTx({ to: batcherContractAddress, value: '3000', data: encodeBatch(recipients) }); + const result = await verify({ + caller: { type: 'transfer', recipients: recipients.map((r) => ({ ...r })) }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a native batch whose value is not the sum of the intent amounts', async function () { + const txHex = await buildTx({ to: batcherContractAddress, value: '2999', data: encodeBatch(recipients) }); + await verify({ + caller: { type: 'transfer', recipients: recipients.map((r) => ({ ...r })) }, + txHex, + }).should.be.rejectedWith(/sum of intent amounts/); + }); + + it('rejects a native batch whose entry does not match the intent', async function () { + const txHex = await buildTx({ to: batcherContractAddress, value: '3000', data: encodeBatch(recipients) }); + await verify({ + caller: { + type: 'transfer', + recipients: [ + { address: recipientAddress, amount: '1000' }, + { address: recipientAddress, amount: '2000' }, + ], + }, + txHex, + }).should.be.rejectedWith(/batch calldata entry 1 does not match intent recipient/); + }); + + it('rejects a native batch sent somewhere other than the batcher', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '3000', data: encodeBatch(recipients) }); + await verify({ + caller: { type: 'transfer', recipients: recipients.map((r) => ({ ...r })) }, + txHex, + }).should.be.rejectedWith(/does not match the network batcher/); + }); + + it('verifies a token batch that matches the declared token', async function () { + const txHex = await buildTx({ + to: batcherContractAddress, + value: '0', + data: encodeBatchTransferFrom(tokenContractAddress, recipients), + }); + const result = await verify({ + caller: { + type: 'transfer', + tokenName, + recipients: recipients.map((r) => ({ ...r })), + }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + }); + + describe('unimplemented intents and malformed bytes', function () { + it('rejects contractCall outright', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '0' }); + await verify({ + caller: { type: 'contractCall' }, + txParams: { type: 'contractCall' }, + txHex, + }).should.be.rejectedWith(/unsupported intent type for EVM TSS/); + }); + + it('validates the EVM encoding of an intent without rules and warns', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '0' }); + const result = await verify({ + caller: { type: 'enableToken' }, + txParams: { type: 'enableToken' }, + txHex, + }); + result.should.equal(true); + warnings + .some((warning) => /no fail-closed verifier for intent type 'enableToken'/.test(warning)) + .should.equal(true); + }); + + it('rejects malformed bytes even when the intent type has no rules', async function () { + await verify({ + caller: { type: 'enableToken' }, + txParams: { type: 'enableToken' }, + txHex: '0x1234', + }).should.be.rejectedWith(/could not be decoded as an EVM transaction/); + }); + + it('rejects non-EVM bytes even when the intent type has no rules', async function () { + // No generic format sniffing here: a coin that owns a non-EVM format handles it in its own + // verifier override. Anything reaching this verifier must decode as an EVM transaction. + await verify({ + caller: { type: 'import', recipients: [{ address: recipientAddress, amount: '1' }] }, + txHex: '0x0000000000000001', + }).should.be.rejected(); + }); + }); + + describe('consolidations', function () { + const otherTokenName = 'hteth:tusdt'; + const otherTokenContractAddress = String( + (coins.get(otherTokenName) as ContractAddressDefinedToken).contractAddress + ); + + /** The recipient shape WP persists for a token consolidation: identity lives in `tokenData`. */ + const tokenIntentRecipient = (amount: string, intentTokenName: string = tokenName) => ({ + address: { address: baseAddress }, + amount: { value: amount, symbol: chain }, + tokenData: { tokenName: intentTokenName, tokenQuantity: amount, tokenType: 'ERC20' }, + }); + + /** + * The effective params the signing path derives from a consolidation intent + * (`resolveEffectiveTxParams` sets the type from `intent.intentType` and back-fills the + * recipients). Without them the missing-params guard rejects the call, which stays true for + * token consolidations exactly as it was before they were recognised. + */ + const effectiveTokenParams = () => ({ + type: 'consolidateToken', + recipients: [{ address: baseAddress, amount: '1000' }], + }); + + it('verifies a native consolidation to the wallet base address', async function () { + const txHex = await buildTx({ to: baseAddress, value: '1000' }); + const result = await verify({ caller: { type: 'consolidate' }, txParams: { type: 'consolidate' }, txHex }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a native consolidation that does not pay the base address', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await verify({ + caller: { type: 'consolidate' }, + txParams: { type: 'consolidate' }, + txHex, + }).should.be.rejectedWith(/base address/); + }); + + it('rejects calldata when a field declares the chain asset', async function () { + // Naming the chain coin is a native declaration: a native sweep is a plain send. + const data = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: baseAddress, value: '1000', data }); + await verify({ + caller: { type: 'consolidate', tokenName: chain }, + txParams: { type: 'consolidate', tokenName: chain }, + txHex, + }).should.be.rejectedWith(/must not carry calldata/); + }); + + it('warns for arbitrary calldata when the consolidation declares no asset', async function () { + // A generic `consolidate` may be an ERC-20 or specialised sweep (the v6 server builds token + // sweeps on this type), so arbitrary calldata is reported rather than called a mismatch. + const data = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: baseAddress, value: '1000', data }); + const result = await verify({ caller: { type: 'consolidate' }, txParams: { type: 'consolidate' }, txHex }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/declares no asset/); + }); + + it('verifies an ERC-20 consolidation on the token the caller declares', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('warns that the token contract is unchecked when no token is declared', async function () { + // Caller-compatible shape: a generic `consolidate` with ERC-20 calldata declares no token + // contract, so the contract is not claimed as verified — but the base address, the calldata + // shape and the zero native value are. + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await verify({ caller: { type: 'consolidate' }, txParams: { type: 'consolidate' }, txHex }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/consolidate: consolidation declares no token contract/); + warnings[0].should.match(/the token contract cannot be checked/); + warnings[0].should.not.match(/no fail-closed verifier/); + }); + + it('rejects a wrong recipient and a nonzero value even with no token metadata', async function () { + // The token contract cannot be checked without metadata, but that must never downgrade the + // checks that do not need it: a wrong sweep recipient and a native value riding along stay + // definite mismatches, and nothing is reported as unverifiable before they are enforced. + const wrongRecipientHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(attackerAddress, '1000'), + }); + await verify({ + caller: { type: 'consolidateToken' }, + txParams: effectiveTokenParams(), + txHex: wrongRecipientHex, + }).should.be.rejectedWith(/Consolidation transaction recipient does not match wallet base address/); + warnings.should.have.length(0); + + const nativeValueHex = await buildTx({ + to: tokenContractAddress, + value: '1', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + await verify({ + caller: { type: 'consolidateToken' }, + txParams: effectiveTokenParams(), + txHex: nativeValueHex, + }).should.be.rejectedWith(/Consolidation token transfer must not carry native value/); + warnings.should.have.length(0); + }); + + it('rejects an ERC-20 consolidation that calls a different token contract', async function () { + const txHex = await buildTx({ + to: attackerAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/does not call the declared token contract/); + }); + + it('rejects an ERC-20 consolidation carrying native value', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '1', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/native value/); + }); + + it('rejects ERC-20 consolidation calldata with trailing words', async function () { + const data = `${encodeErc20Transfer(baseAddress, '1000')}${'00'.repeat(32)}`; + const txHex = await buildTx({ to: tokenContractAddress, value: '0', data }); + await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/not canonical/); + }); + + it('never compares the swept amount', async function () { + // A consolidation moves whatever the address holds: the calldata amount is not intent data. + const swept = '123456789012345678901234567890'; + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, swept), + }); + const result = await verify({ + intent: { intentType: 'consolidateToken', recipients: [tokenIntentRecipient('1')] }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('verifies a stored consolidateToken intent without a consolidation id', async function () { + // The signing path derives `txParams.type` from `intent.intentType`; a missing consolidate id + // must not downgrade the intent to "unknown type". + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await verify({ + intent: { intentType: 'consolidateToken', recipients: [tokenIntentRecipient('1000')] }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a stored consolidateToken intent that pays a different recipient', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(attackerAddress, '1000'), + }); + await verify({ + intent: { intentType: 'consolidateToken', recipients: [tokenIntentRecipient('1000')] }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/Consolidation transaction recipient does not match wallet base address/); + }); + + it('rejects a stored consolidateToken intent that calls a different token contract', async function () { + const txHex = await buildTx({ + to: otherTokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + await verify({ + intent: { intentType: 'consolidateToken', recipients: [tokenIntentRecipient('1000')] }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/does not call the declared token contract/); + }); + + it('warns instead of guessing when the stored token name cannot be resolved', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await verify({ + intent: { + intentType: 'consolidateToken', + recipients: [tokenIntentRecipient('1000', 'hteth:notaregisteredtoken')], + }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/consolidateToken: .*cannot be checked/); + warnings[0].should.not.match(/no fail-closed verifier/); + // Console output reaches log pipelines: declared/decoded addresses and calldata stay out of it. + warnings[0].should.not.match(/0x[0-9a-fA-F]{40}/); + warnings[0].should.not.match(/expected|decoded/); + }); + + it('prefers the caller token declaration over the stored one', async function () { + // Stored intent names a different token; the caller names the one the bytes target. + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await verify({ + caller: { type: 'consolidateToken', tokenName }, + intent: { intentType: 'consolidateToken', recipients: [tokenIntentRecipient('1000', otherTokenName)] }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a consolidateToken intent that carries no transfer calldata', async function () { + const txHex = await buildTx({ to: baseAddress, value: '1000' }); + await verify({ + caller: { type: 'consolidateToken' }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/carries no ERC-20 transfer/); + }); + + it('keeps rejecting a consolidateToken call with no recipients and no consolidate id', async function () { + // Recognising the type must not relax the missing-params guard: with no recipients and no + // consolidate id there is nothing to verify the bytes against, so this stays a rejection + // rather than becoming a warning on the way to signing. + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + await verify({ + caller: { type: 'consolidateToken' }, + txParams: { type: 'consolidateToken' }, + txHex, + }).should.be.rejectedWith(/missing txParams/); + // A stored intent alone does not supply the params the guard requires either. + await verify({ + intent: { intentType: 'consolidateToken', recipients: [tokenIntentRecipient('1000')] }, + txParams: { type: 'consolidateToken' }, + txHex, + }).should.be.rejectedWith(/missing txParams/); + }); + + it('rejects malformed consolidation bytes even when the base address is unavailable', async function () { + await verify({ + caller: { type: 'consolidate' }, + txParams: { type: 'consolidate' }, + txHex: '0x1234', + omitBaseAddress: true, + }).should.be.rejected(); + }); + + it('decodes a consolidation before warning that the base address is unavailable', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '1000' }); + const result = await verify({ + caller: { type: 'consolidate' }, + txParams: { type: 'consolidate' }, + txHex, + omitBaseAddress: true, + }); + result.should.equal(true); + warnings.some((warning) => /base address is not available/.test(warning)).should.equal(true); + }); + + it('rejects batch calldata when a field declares the chain asset', async function () { + const batchData = encodeBatch([{ address: recipientAddress, amount: '1000' }]); + const txHex = await buildTx({ to: batcherContractAddress, value: '1000', data: batchData }); + await verify({ + caller: { type: 'consolidate', tokenName: chain }, + txParams: { type: 'consolidate', tokenName: chain }, + txHex, + }).should.be.rejectedWith(/must not carry calldata/); + warnings.should.have.length(0); + }); + + it('warns for batch calldata when the consolidation declares no asset', async function () { + // With no asset declared the batch could belong to a token or specialised sweep, so it is + // reported as an unimplemented shape instead of being called a mismatch. + const batchData = encodeBatch([{ address: recipientAddress, amount: '1000' }]); + const txHex = await buildTx({ to: batcherContractAddress, value: '1000', data: batchData }); + const result = await verify({ caller: { type: 'consolidate' }, txParams: { type: 'consolidate' }, txHex }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/\[tss verification\] consolidate: batch consolidation calldata cannot be verified/); + // Console output reaches log pipelines: decoded addresses and calldata stay out of it. + warnings[0].should.not.match(/0x[0-9a-fA-F]{40}/); + warnings[0].should.not.containEql(batchData.slice(10)); + warnings[0].should.not.match(/expected|decoded/); + }); + + it('rejects batch calldata on a token consolidation', async function () { + const txHex = await buildTx({ + to: batcherContractAddress, + value: '0', + data: encodeBatchTransferFrom(tokenContractAddress, [{ address: baseAddress, amount: '1000' }]), + }); + await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/does not use ERC-20 transfer/); + warnings.should.have.length(0); + }); + + it('keeps the historic consolidationToBaseAddress check', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await verify({ + caller: { type: 'transfer', recipients: [{ address: attackerAddress, amount: '1000' }] }, + txHex, + verification: { consolidationToBaseAddress: true }, + }).should.be.rejectedWith(/Consolidation transaction recipient does not match wallet base address/); + }); + + it('keeps every historic flag rejection ahead of any unverifiable report', async function () { + // The flag has always compared `tx.to` with the base address for every calldata that is not + // an ERC-20 transfer, so neither a plain function call nor a batch may be downgraded to a + // warning on this path. + const approveData = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const approveHex = await buildTx({ to: attackerAddress, value: '0', data: approveData }); + await verify({ + caller: { type: 'consolidate' }, + txHex: approveHex, + verification: { consolidationToBaseAddress: true }, + }).should.be.rejectedWith(/Consolidation transaction recipient does not match wallet base address/); + + const batchData = encodeBatch([{ address: baseAddress, amount: '1000' }]); + const batchHex = await buildTx({ to: batcherContractAddress, value: '1000', data: batchData }); + await verify({ + caller: { type: 'consolidate' }, + txHex: batchHex, + verification: { consolidationToBaseAddress: true }, + }).should.be.rejectedWith(/Consolidation transaction recipient does not match wallet base address/); + warnings.should.have.length(0); + }); + + it('keeps the historic flag pass for token-shaped calldata', async function () { + // The flag's ERC-20 branch never compared `to` with the base address, so a v6 token + // consolidation still verifies and reports only the contract it cannot check. + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await verify({ + caller: { type: 'consolidate' }, + txHex, + verification: { consolidationToBaseAddress: true }, + }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/token contract cannot be checked/); + }); + + it('rejects a wrong function on an established token contract', async function () { + // The intent names the token and the transaction calls that contract, but not with + // transfer(address,uint256): the declared token cannot reach the base address this way. + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word((1n << 256n) - 1n)}`; + const txHex = await buildTx({ to: tokenContractAddress, value: '0', data: approve }); + await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/does not use ERC-20 transfer/); + warnings.should.have.length(0); + }); + + it('rejects native value on an unresolved token consolidation with unknown calldata', async function () { + // The contract is unknown, but a token sweep carrying native value is a definite mismatch: + // the missing address must not suppress that check. + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: attackerAddress, value: '5', data: approve }); + await verify({ + caller: { type: 'consolidateToken' }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/must not carry native value/); + warnings.should.have.length(0); + }); + + it('warns for non-transfer calldata while the token contract is unresolved', async function () { + // The contract is unknown, so the shape cannot be anchored to it; the native-value check and + // the zero-value case below are the independent expectations this intent establishes. + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: attackerAddress, value: '0', data: approve }); + const result = await verify({ + caller: { type: 'consolidateToken' }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/token contract nor the calldata can be verified/); + warnings[0].should.not.match(/no fail-closed verifier/); + }); + + it('does not treat another chain coin name as this chain native asset', async function () { + // `btc` is a statics coin, but not this chain's asset: the declaration cannot be trusted to + // mean "native here", so the shape is reported instead of being rejected as native calldata. + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: baseAddress, value: '0', data: approve }); + const result = await verify({ + caller: { type: 'consolidate', tokenName: 'btc' }, + txParams: { type: 'consolidate', tokenName: 'btc' }, + txHex, + }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/asset 'btc' on another network/); + }); + + it('does not adopt a token contract declared on another network', async function () { + // Bare `usdc` resolves in statics to the Ethereum contract; on BSC it names no local asset, + // so it must warn instead of rejecting the BSC token contract as the wrong one. + const bscStatics = coins.get('bsc'); + const txHex = await buildTxFor(bscStatics, { + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await new TestEthLikeCoin(bitgo, bscStatics).verifyTssTransaction({ + callerTxParams: { type: 'consolidateToken', tokenName: 'usdc' }, + txParams: { type: 'consolidateToken', recipients: [{ address: baseAddress, amount: '1000' }] }, + txPrebuild: { txHex }, + wallet: { coinSpecific: () => ({ baseAddress }) }, + } as unknown as Kernel); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/another network/); + warnings[0].should.match(/token contract cannot be checked/); + }); + + it('keeps bare usdc resolvable on its own network', async function () { + const ethStatics = coins.get('eth'); + const usdcContract = String((coins.get('usdc') as ContractAddressDefinedToken).contractAddress); + const txHex = await buildTxFor(ethStatics, { + to: usdcContract, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await new TestEthLikeCoin(bitgo, ethStatics).verifyTssTransaction({ + callerTxParams: { type: 'consolidateToken', tokenName: 'usdc' }, + txParams: { type: 'consolidateToken', recipients: [{ address: baseAddress, amount: '1000' }] }, + txPrebuild: { txHex }, + wallet: { coinSpecific: () => ({ baseAddress }) }, + } as unknown as Kernel); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('verifies a BSC token consolidation on its own network', async function () { + // BSC tokens are BscCoin, not Erc20Coin: the ERC-20 family is established by the statics + // class, not by one concrete class name. + const bscStatics = coins.get('bsc'); + const bscTokenName = 'bsc:busd'; + const bscContract = String((coins.get(bscTokenName) as ContractAddressDefinedToken).contractAddress); + const txHex = await buildTxFor(bscStatics, { + to: bscContract, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await new TestEthLikeCoin(bitgo, bscStatics).verifyTssTransaction({ + callerTxParams: { type: 'consolidateToken', tokenName: bscTokenName }, + txParams: { type: 'consolidateToken', recipients: [{ address: baseAddress, amount: '1000' }] }, + txPrebuild: { txHex }, + wallet: { coinSpecific: () => ({ baseAddress }) }, + } as unknown as Kernel); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a wrong function on a BSC token consolidation', async function () { + const bscStatics = coins.get('bsc'); + const bscTokenName = 'bsc:busd'; + const bscContract = String((coins.get(bscTokenName) as ContractAddressDefinedToken).contractAddress); + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTxFor(bscStatics, { to: bscContract, value: '0', data: approve }); + await new TestEthLikeCoin(bitgo, bscStatics) + .verifyTssTransaction({ + callerTxParams: { type: 'consolidateToken', tokenName: bscTokenName }, + txParams: { type: 'consolidateToken', recipients: [{ address: baseAddress, amount: '1000' }] }, + txPrebuild: { txHex }, + wallet: { coinSpecific: () => ({ baseAddress }) }, + } as unknown as Kernel) + .should.be.rejectedWith(/does not use ERC-20 transfer/); + }); + + it('verifies an XDC token consolidation on its own network', async function () { + const xdcStatics = coins.get('xdc'); + const xdcTokenName = 'xdc:usdc'; + const xdcContract = String((coins.get(xdcTokenName) as ContractAddressDefinedToken).contractAddress); + const txHex = await buildTxFor(xdcStatics, { + to: xdcContract, + value: '0', + data: encodeErc20Transfer(baseAddress, '1000'), + }); + const result = await new TestEthLikeCoin(bitgo, xdcStatics).verifyTssTransaction({ + callerTxParams: { type: 'consolidateToken', tokenName: xdcTokenName }, + txParams: { type: 'consolidateToken', recipients: [{ address: baseAddress, amount: '1000' }] }, + txPrebuild: { txHex }, + wallet: { coinSpecific: () => ({ baseAddress }) }, + } as unknown as Kernel); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects a wrong function on an XDC token consolidation', async function () { + const xdcStatics = coins.get('xdc'); + const xdcTokenName = 'xdc:usdc'; + const xdcContract = String((coins.get(xdcTokenName) as ContractAddressDefinedToken).contractAddress); + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTxFor(xdcStatics, { to: xdcContract, value: '0', data: approve }); + await new TestEthLikeCoin(bitgo, xdcStatics) + .verifyTssTransaction({ + callerTxParams: { type: 'consolidateToken', tokenName: xdcTokenName }, + txParams: { type: 'consolidateToken', recipients: [{ address: baseAddress, amount: '1000' }] }, + txPrebuild: { txHex }, + wallet: { coinSpecific: () => ({ baseAddress }) }, + } as unknown as Kernel) + .should.be.rejectedWith(/does not use ERC-20 transfer/); + }); + + it('warns for a known NFT instead of forcing it through ERC-20', async function () { + // Erc721Coin also extends ContractAddressDefinedToken; its standard is unclassified, so a + // transfer-shaped call is reported rather than accepted as an ERC-20 sweep. + const nftChain = 'thbarevm'; + const nftStatics = coins.get(nftChain); + const nftTokenName = 'thbarevm:tmnft'; + const nftContract = String((coins.get(nftTokenName) as ContractAddressDefinedToken).contractAddress); + const txHex = await buildTxFor(nftStatics, { + to: nftContract, + value: '0', + data: encodeErc20Transfer(baseAddress, '1'), + }); + const result = await new TestEthLikeCoin(bitgo, nftStatics).verifyTssTransaction({ + callerTxParams: { type: 'consolidateToken', tokenName: nftTokenName }, + txParams: { type: 'consolidateToken', recipients: [{ address: baseAddress, amount: '1' }] }, + txPrebuild: { txHex }, + wallet: { coinSpecific: () => ({ baseAddress }) }, + } as unknown as Kernel); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/not established as ERC-20/); + }); + + it('reports, not rejects, native value on a specialised token shape', async function () { + const specialisedToken = 'hteth:ctest1'; + const specialisedContract = String((coins.get(specialisedToken) as ContractAddressDefinedToken).contractAddress); + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: specialisedContract, value: '5', data: approve }); + const result = await verify({ + caller: { type: 'consolidateToken', tokenName: specialisedToken }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/specialised token consolidation/); + }); + + it('does not reject a specialised token consolidation it does not model', async function () { + // ERC-7984 sweeps are verified by the token coin's own verifier; a format this verifier does + // not own is reported here, never rejected. + const specialisedToken = 'hteth:ctest1'; + const specialisedContract = String((coins.get(specialisedToken) as ContractAddressDefinedToken).contractAddress); + const approve = `0x095ea7b3${addrWord(attackerAddress)}${word(1n)}`; + const txHex = await buildTx({ to: specialisedContract, value: '0', data: approve }); + const result = await verify({ + caller: { type: 'consolidateToken', tokenName: specialisedToken }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/specialised token consolidation/); + }); + + it('warns for batch calldata while the token contract is unresolved', async function () { + const txHex = await buildTx({ + to: batcherContractAddress, + value: '0', + data: encodeBatchTransferFrom(tokenContractAddress, [{ address: baseAddress, amount: '1000' }]), + }); + const result = await verify({ + caller: { type: 'consolidateToken' }, + txParams: effectiveTokenParams(), + txHex, + }); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].should.match(/batch consolidation calldata cannot be verified/); + }); + + it('rejects native value on an unresolved token batch', async function () { + // Native value on a declared token does not depend on knowing the contract: the non-batch + // branch already rejects it, and the batch encoding must not be a way around that. + const txHex = await buildTx({ + to: batcherContractAddress, + value: '5', + data: encodeBatchTransferFrom(tokenContractAddress, [{ address: baseAddress, amount: '1000' }]), + }); + await verify({ + caller: { type: 'consolidateToken' }, + txParams: effectiveTokenParams(), + txHex, + }).should.be.rejectedWith(/must not carry native value/); + warnings.should.have.length(0); + }); + + it('rejects native value and any batch encoding on a token batch', async function () { + const recipients = [{ address: baseAddress, amount: '1000' }]; + const withValueHex = await buildTx({ + to: batcherContractAddress, + value: '5', + data: encodeBatchTransferFrom(tokenContractAddress, recipients), + }); + await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex: withValueHex, + }).should.be.rejectedWith(/must not carry native value/); + + const zeroValueHex = await buildTx({ + to: batcherContractAddress, + value: '0', + data: encodeBatchTransferFrom(tokenContractAddress, recipients), + }); + await verify({ + caller: { type: 'consolidateToken', tokenName }, + txParams: effectiveTokenParams(), + txHex: zeroValueHex, + }).should.be.rejectedWith(/does not use ERC-20 transfer/); + warnings.should.have.length(0); + }); + }); + + describe('assertSignableConsistency envelope handling', function () { + const tbscCoin = coins.get('tbsc'); + const tbscChainId = (tbscCoin.network as EthLikeNetwork).chainId; + + it('parses a legacy type-0 envelope on an EIP-1559 chain and compares its preimage', async function () { + const txHex = await buildTxFor(staticsCoin, { to: recipientAddress, value: '1000' }); + txHex.startsWith('0x02').should.equal(false); + (() => coin.assertSignableConsistency(txHex, 'deadbeef')).should.throw(/signableHex is inconsistent/); + }); + + it('parses a type-2 envelope on an EIP-1559 chain and compares its preimage', async function () { + const txHex = await buildTxFor(staticsCoin, { to: recipientAddress, value: '1000', eip1559: true }); + txHex.startsWith('0x02').should.equal(true); + (() => coin.assertSignableConsistency(txHex, 'deadbeef')).should.throw(/signableHex is inconsistent/); + }); + + it('uses the base chain fork for a token coin on an EIP-1559 chain', async function () { + const tokenCoin = new TestEthLikeCoin(bitgo, coins.get(tokenName)); + const txHex = await buildTxFor(coins.get(tokenName), { to: recipientAddress, value: '1000', eip1559: true }); + (() => tokenCoin.assertSignableConsistency(txHex, 'deadbeef')).should.throw(/signableHex is inconsistent/); + }); + + it('parses a legacy type-0 envelope on a chain without EIP-1559', async function () { + const legacyCoin = new TestEthLikeCoin(bitgo, tbscCoin); + const txHex = await buildTxFor(tbscCoin, { to: recipientAddress, value: '1000' }); + (() => legacyCoin.assertSignableConsistency(txHex, 'deadbeef')).should.throw(/signableHex is inconsistent/); + }); + + it('rejects a type-2 envelope on a chain without EIP-1559 before comparing preimages', function () { + const legacyCoin = new TestEthLikeCoin(bitgo, tbscCoin); + const common = EthereumCommon.forCustomChain( + 'mainnet', + { name: 'tbsc', networkId: tbscChainId, chainId: tbscChainId }, + 'london' + ); + const typed = FeeMarketEIP1559Transaction.fromTxData( + { + nonce: 1, + gasLimit: '0x186a0', + maxFeePerGas: '0x0a', + maxPriorityFeePerGas: '0x01', + to: recipientAddress, + value: '0x3e8', + data: '0x', + chainId: tbscChainId, + }, + { common } + ); + (() => legacyCoin.assertSignableConsistency(`0x${typed.serialize().toString('hex')}`, 'deadbeef')).should.throw( + /EIP-1559 not enabled/ + ); + }); + }); +}); diff --git a/modules/sdk-coin-bsc/test/unit/bsc.ts b/modules/sdk-coin-bsc/test/unit/bsc.ts index 6badbedd83b..98faf34d859 100644 --- a/modules/sdk-coin-bsc/test/unit/bsc.ts +++ b/modules/sdk-coin-bsc/test/unit/bsc.ts @@ -130,7 +130,7 @@ describe('Native BNB', function () { const result = await coin.verifyTssTransaction({ txParams: { type: 'transfer', - recipients: [{ address: recipientAddress, amount: '10000000' }], + recipients: [{ address: recipientAddress, amount: '10000000', tokenAddress: tokenContractAddress }], } as any, txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any, wallet, diff --git a/modules/sdk-coin-bsc/test/unit/tssSigningVerification.ts b/modules/sdk-coin-bsc/test/unit/tssSigningVerification.ts new file mode 100644 index 00000000000..081ad9aabdd --- /dev/null +++ b/modules/sdk-coin-bsc/test/unit/tssSigningVerification.ts @@ -0,0 +1,390 @@ +import 'should'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGo } from '@bitgo/sdk-test'; +import { coins, ContractAddressDefinedToken, EthereumNetwork } from '@bitgo/statics'; +import { + EcdsaMPCv2Utils, + EcdsaUtils, + InvalidTransactionError, + PopulatedIntent, + RequestTracer, + TransactionParams, + TransactionType, + TxIntentMismatchError, + TxIntentMismatchRecipientError, + TxRequest, + Wallet, +} from '@bitgo/sdk-core'; +import { getCommon, optionalDeps } from '@bitgo/abstract-eth'; + +import { BscToken, Tbsc } from '../../src'; +import { TransactionBuilder } from '../../src/lib'; + +/** + * CECHO-2460 / INFOSEC-671 — the user key share must not sign BSC bytes that were never compared + * with the intent the caller declared. + * + * BSC has no EIP-1559, so it takes the legacy type-0 envelope and the + * `CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX` branch: the wallet platform sends the unsigned + * broadcast-format transaction as `serializedTxHex` and the EIP-155 preimage + * `rlp([nonce, gasPrice, gasLimit, to, value, data, chainId, 0, 0])` as `signableHex`. The verifier + * must decode the bytes the transaction actually carries (the serialized ones), and the signing + * round must only be reached with a preimage derived from those same bytes. + * + * The cases drive the real signing entries (`EcdsaUtils.signTxRequest` and + * `EcdsaMPCv2Utils.signTxRequest`), the real verifier and the real transaction builder; the signing + * round itself is replaced by a sentinel so a rejection can be proven to stop it. + * + * Policy pinned by these cases: any definite mismatch between the bytes and the user intent throws + * in the single default mode — destination, amount, wrong or unexpected calldata, and a payment + * declared without recipients. Only a finding the verifier cannot decide locally (an intent type + * with no validator) warns and lets signing proceed. + */ + +const walletId = '62fe536a6b4cf70007acb48c0e7bb0b0'; +const baseAddress = '0x1111111111111111111111111111111111111111'; +const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; +const attackerAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; +const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; +const transferAmount = '1000'; +const approveAttackerMax = `0x095ea7b3${attackerAddress.slice(2).padStart(64, '0')}${'f'.repeat(64)}`; +// Minimal signing material: signRequestBase only needs pShare.i === 1, a backupNShare and the +// paillier modulus before it hands the unsigned bytes to verifyTransaction. +const prv = JSON.stringify({ pShare: { i: 1, n: 'paillier-modulus' }, backupNShare: {} }); +// MPCv2 takes the user key share as base64; the verifier runs before any share is used. +const mpcv2Prv = Buffer.from('mpcv2-user-key-share').toString('base64'); + +/** ERC-20 `transfer(address,uint256)` calldata, encoded without extra test dependencies. */ +const encodeErc20Transfer = (to: string, amount: string): string => { + const address = to.toLowerCase().replace(/^0x/, '').padStart(64, '0'); + const value = BigInt(amount).toString(16).padStart(64, '0'); + return `0xa9059cbb${address}${value}`; +}; + +interface SigningMaterial { + /** Broadcast-format unsigned transaction, without the 0x prefix as the wallet platform sends it. */ + serializedTxHex: string; + /** The EIP-155 preimage the user key share signs. */ + signableHex: string; +} + +interface TxShape { + to: string; + value: string; + data?: string; +} + +const buildSigningMaterial = async (tx: TxShape): Promise => { + const staticsCoin = coins.get('tbsc'); + const builder = new TransactionBuilder(staticsCoin); + builder.type(tx.data ? TransactionType.ContractCall : TransactionType.SingleSigSend); + builder.fee({ fee: '30', gasPrice: '30', gasLimit: '21000' }); + builder.counter(1); + builder.contract(tx.to); + if (tx.data) { + builder.data(tx.data); + } + builder.value(tx.value); + + const broadcast = (await builder.build()).toBroadcastFormat(); + const serializedTxHex = broadcast.startsWith('0x') ? broadcast.slice(2) : broadcast; + + const common = getCommon(staticsCoin.network as EthereumNetwork); + // BSC never activated EIP-1559; it signs under the petersburg hardfork. + common.setHardfork(optionalDeps.EthCommon.Hardfork.Petersburg); + // BSC only ever builds the legacy EIP-155 envelope; its preimage is + // [nonce, gasPrice, gasLimit, to, value, data, chainId, 0, 0]. + const legacyTx = optionalDeps.EthTx.Transaction.fromSerializedTx(Buffer.from(serializedTxHex, 'hex'), { common }); + const signableHex = Buffer.from(optionalDeps.ethUtil.rlp.encode(legacyTx.getMessageToSign(false))).toString('hex'); + + return { serializedTxHex, signableHex }; +}; + +interface SignOutcome { + /** True only when execution got past verification and into the signing round. */ + signingStarted: boolean; + error?: Error; + /** Whatever the verifier reported through `console.warn` while deciding. */ + warnings: string[]; +} + +/** Runs the real ECDSA or MPCv2 signing path and reports whether the signing round began. */ +const sign = async (opts: { + material: SigningMaterial; + txParams?: TransactionParams; + intent?: PopulatedIntent; + /** Coin instance the signing request is routed through; defaults to the native tbsc coin. */ + coinName?: string; + implementation?: 'ecdsa' | 'mpcv2'; +}): Promise => { + const implementation = opts.implementation ?? 'ecdsa'; + const bitgo = TestBitGo.decorate(BitGoAPI, { env: 'test' }); + bitgo.safeRegister('tbsc', Tbsc.createInstance); + BscToken.createTokenConstructors().forEach(({ name, coinConstructor }) => bitgo.safeRegister(name, coinConstructor)); + bitgo.initializeTestVars(); + + const coin = bitgo.coin(opts.coinName ?? 'tbsc'); + const wallet = new Wallet(bitgo, coin, { id: walletId, coinSpecific: { baseAddress }, multisigType: 'tss' }); + const utils = + implementation === 'mpcv2' ? new EcdsaMPCv2Utils(bitgo, coin, wallet) : new EcdsaUtils(bitgo, coin, wallet); + + // Replace the first step after verification with a sentinel: reaching it means the verifier let + // these bytes through to the signing round. + let signingStarted = false; + if (implementation === 'mpcv2') { + const mpcv2 = utils as unknown as { + pickBitgoPubGpgKeyForSigning: () => Promise; + getMpcv2HashBuffer: () => Buffer; + }; + // The BitGo GPG key lookup is not the signing round; keep the suite offline. + mpcv2.pickBitgoPubGpgKeyForSigning = async () => ({}); + mpcv2.getMpcv2HashBuffer = () => { + signingStarted = true; + throw new Error('stop: signing round started'); + }; + } else { + const ecdsa = utils as unknown as { getEcdsaSigningChallenges: () => Promise }; + ecdsa.getEcdsaSigningChallenges = async () => { + signingStarted = true; + throw new Error('stop: signing round started'); + }; + } + + // Only the fields the signing path reads are populated; TxRequest carries many server fields. + const txRequest = { + txRequestId: 'signing-path', + apiVersion: 'full', + walletId, + intent: opts.intent, + transactions: [ + { + unsignedTx: { + derivationPath: 'm/0', + signableHex: opts.material.signableHex, + serializedTxHex: opts.material.serializedTxHex, + }, + signatureShares: [], + }, + ], + } as unknown as TxRequest; + + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { + warnings.push(args.map((arg) => String(arg)).join(' ')); + }; + try { + await utils.signTxRequest({ + txRequest, + txParams: opts.txParams, + prv: implementation === 'mpcv2' ? mpcv2Prv : prv, + reqId: new RequestTracer(), + }); + } catch (e) { + return { signingStarted, error: e as Error, warnings }; + } finally { + console.warn = originalWarn; + } + return { signingStarted, warnings }; +}; + +/** The intent as the wallet platform stores it on a tx request. */ +const storedIntent = (intentType: string, address: string, value: string): PopulatedIntent => ({ + intentType, + recipients: [{ address: { address }, amount: { value, symbol: 'tbsc' } }], +}); + +const callerTransfer = (address: string, amount: string): TransactionParams => ({ + type: 'transfer', + recipients: [{ address, amount }], +}); + +const expectRejectedWithoutSigning = (outcome: SignOutcome): void => { + outcome.signingStarted.should.equal(false, 'the signing round must never start'); + (outcome.error as Error).should.be.instanceOf(Error); + (outcome.error as Error).message.should.not.match(/signing round started/); +}; + +const expectSigned = (outcome: SignOutcome): void => { + outcome.signingStarted.should.equal(true, `expected the signing round to start, got: ${outcome.error?.message}`); + (outcome.error as Error).message.should.match(/signing round started/); +}; + +describe('BSC TSS signing path verifies the legacy bytes before signing (CECHO-2460)', function () { + it('signs a payment whose serialized bytes match the intent, and signs the matching preimage', async function () { + const material = await buildSigningMaterial({ to: recipientAddress, value: transferAmount }); + // Legacy chains send two different hexes: the broadcast bytes and the EIP-155 preimage. + material.signableHex.should.not.equal( + material.serializedTxHex, + 'the preimage and the broadcast bytes must be distinct, as the platform sends them' + ); + const outcome = await sign({ material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectSigned(outcome); + }); + + it('never starts signing a payment redirected in the serialized bytes', async function () { + // A parse error instead of the recipient mismatch would mean the verifier decoded the EIP-155 + // preimage (v = chainId) rather than the serialized transaction the user is signing. + const material = await buildSigningMaterial({ to: attackerAddress, value: transferAmount }); + const outcome = await sign({ material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + it('never starts signing a payment the serialized bytes inflate', async function () { + const material = await buildSigningMaterial({ to: recipientAddress, value: '2000' }); + const outcome = await sign({ material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/does not match the value given by client/); + }); + + it('never starts signing a preimage that was not derived from the verified serialized bytes', async function () { + const benign = await buildSigningMaterial({ to: recipientAddress, value: transferAmount }); + const attacker = await buildSigningMaterial({ to: attackerAddress, value: transferAmount }); + // Verify the benign bytes, then sign the attacker's preimage. + const material = { serializedTxHex: benign.serializedTxHex, signableHex: attacker.signableHex }; + const outcome = await sign({ material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(InvalidTransactionError); + (outcome.error as Error).message.should.match(/signableHex is inconsistent with serializedTxHex/); + }); + + it('never starts signing a caller-declared payment that declares no recipients', async function () { + // The stored intent is server data: it must not fill the gap and decide the destination. + const material = await buildSigningMaterial({ to: attackerAddress, value: transferAmount }); + const outcome = await sign({ + material, + txParams: { type: 'transfer' }, + intent: storedIntent('payment', attackerAddress, transferAmount), + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/without recipients/); + }); + + it('signs a stored-intent payment the caller did not re-declare (pending approval)', async function () { + const material = await buildSigningMaterial({ to: recipientAddress, value: transferAmount }); + const outcome = await sign({ material, intent: storedIntent('payment', recipientAddress, transferAmount) }); + expectSigned(outcome); + }); + + it('never starts signing stored-intent bytes that redirect the payment', async function () { + const material = await buildSigningMaterial({ to: attackerAddress, value: transferAmount }); + const outcome = await sign({ material, intent: storedIntent('payment', recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + }); + + it('verifies the ERC-20 recipient inside the serialized calldata, not just the outer to', async function () { + const material = await buildSigningMaterial({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(attackerAddress, transferAmount), + }); + const outcome = await sign({ material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + describe('token coin route (tbsc:usd1)', function () { + const tokenCoinName = 'tbsc:usd1'; + const tokenStaticsCoin = coins.get(tokenCoinName); + if (!(tokenStaticsCoin instanceof ContractAddressDefinedToken)) { + throw new Error(`${tokenCoinName} must be a contract-address token in statics`); + } + const tokenContract = String(tokenStaticsCoin.contractAddress); + const callerTokenTransfer = (address: string, amount: string): TransactionParams => ({ + type: 'transfer', + recipients: [{ address, amount, tokenName: tokenCoinName }], + }); + + it('signs a token transfer whose serialized calldata matches the declared token and recipient', async function () { + const material = await buildSigningMaterial({ + to: tokenContract, + value: '0', + data: encodeErc20Transfer(recipientAddress, transferAmount), + }); + const outcome = await sign({ + material, + txParams: callerTokenTransfer(recipientAddress, transferAmount), + coinName: tokenCoinName, + }); + expectSigned(outcome); + }); + + it('never starts signing a token transfer whose calldata recipient is swapped', async function () { + const material = await buildSigningMaterial({ + to: tokenContract, + value: '0', + data: encodeErc20Transfer(attackerAddress, transferAmount), + }); + const outcome = await sign({ + material, + txParams: callerTokenTransfer(recipientAddress, transferAmount), + coinName: tokenCoinName, + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + }); + + it('never starts signing approve(attacker, MAX) labelled as a payment', async function () { + const material = await buildSigningMaterial({ to: tokenContractAddress, value: '0', data: approveAttackerMax }); + const outcome = await sign({ material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchError); + }); + + it('never starts signing a stored contractCall intent it has no verifier for', async function () { + // `contractCall` / `customTx` carry arbitrary calldata with no intent shape to verify against, + // so they are refused outright instead of being signed unchecked. + const material = await buildSigningMaterial({ to: tokenContractAddress, value: '0', data: approveAttackerMax }); + const outcome = await sign({ material, intent: storedIntent('contractCall', tokenContractAddress, '0') }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).message.should.match(/unsupported intent type for EVM TSS: contractCall/); + }); + + describe('MPCv2 signing path', function () { + const implementation = 'mpcv2'; + + it('signs a payment whose serialized bytes match the intent', async function () { + const material = await buildSigningMaterial({ to: recipientAddress, value: transferAmount }); + const outcome = await sign({ + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectSigned(outcome); + }); + + it('never starts signing a payment the serialized bytes redirect', async function () { + const material = await buildSigningMaterial({ to: attackerAddress, value: transferAmount }); + const outcome = await sign({ + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + it('never starts signing a preimage that was not derived from the verified bytes', async function () { + const benign = await buildSigningMaterial({ to: recipientAddress, value: transferAmount }); + const attacker = await buildSigningMaterial({ to: attackerAddress, value: transferAmount }); + const material = { serializedTxHex: benign.serializedTxHex, signableHex: attacker.signableHex }; + const outcome = await sign({ + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(InvalidTransactionError); + (outcome.error as Error).message.should.match(/signableHex is inconsistent with serializedTxHex/); + }); + }); +}); diff --git a/modules/sdk-coin-eth/test/unit/eth.ts b/modules/sdk-coin-eth/test/unit/eth.ts index 9eeb9106d37..f0321d081a1 100644 --- a/modules/sdk-coin-eth/test/unit/eth.ts +++ b/modules/sdk-coin-eth/test/unit/eth.ts @@ -78,6 +78,21 @@ describe('ETH:', function () { return (await txBuilder.build()).toBroadcastFormat(); }; + /** + * Build the serialized bytes of a real native-coin send. Consolidation, acceleration and + * fillNonce intents are verified against these bytes, so they must be a decodable EVM + * transaction rather than a placeholder. + */ + const buildNativeTxHex = async (to: string, value = '0'): Promise => { + const txBuilder = getBuilder('hteth') as TransactionBuilder; + txBuilder.type(TransactionType.SingleSigSend); + txBuilder.fee({ fee: '10', gasLimit: '21000' }); + txBuilder.counter(1); + txBuilder.contract(to); + txBuilder.value(value); + return (await txBuilder.build()).toBroadcastFormat(); + }; + const defiCalldata = (signature: string, values: string[]): string => { const types = signature.slice(signature.indexOf('(') + 1, -1).split(','); const methodId = EthereumAbi.methodID(signature.split('(')[0], types); @@ -706,6 +721,44 @@ describe('ETH:', function () { throw new Error('expected verifyTransaction to be rejected, but it resolved'); }; + /** + * Run a verification while collecting the verifier's warning lines, so a test can tell + * "the bytes were checked" apart from "the intent type has no fail-closed verifier". + */ + const withVerificationWarnings = async ( + run: () => Promise + ): Promise<{ result: boolean; warnings: string[] }> => { + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]): void => { + warnings.push(args.map((arg) => String(arg)).join(' ')); + }; + try { + return { result: await run(), warnings }; + } finally { + console.warn = originalWarn; + } + }; + + /** + * Run a fixture through the coin's real TSS signing entry point. The fixtures are the loose + * shapes that path passes: no caller recipients for intent-only flows, and a prebuild that + * carries only the fields the verifier reads. + */ + const verifyTssFixture = ( + txParams: Record, + txPrebuild: Record, + wallet: Wallet, + verification: Record = {} + ): Promise => + (bitgo.coin('hteth') as Hteth).verifyTransaction({ + txParams: txParams as unknown as VerifyEthTransactionOptions['txParams'], + txPrebuild: txPrebuild as unknown as VerifyEthTransactionOptions['txPrebuild'], + wallet, + verification: verification as unknown as VerifyEthTransactionOptions['verification'], + walletType: 'tss', + }); + it('should verify TSS consolidation transaction when txPrebuild has consolidateId', async function () { const coin = bitgo.coin('hteth') as Hteth; const baseAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; @@ -724,21 +777,17 @@ describe('ETH:', function () { // txPrebuild with consolidateId (set by server during consolidation build) const txPrebuild = { consolidateId: '68a7d5d0c66e74e216b97173bd558c6d', - txHex: '0x', + // A sweep of the address balance to the wallet base address. + txHex: await buildNativeTxHex(baseAddress, '1000000000000000000'), coin: 'hteth', walletId: 'fakeWalletId', }; - const verification = {}; - - const isTransactionVerified = await coin.verifyTransaction({ - txParams: txParams as any, - txPrebuild: txPrebuild as any, - wallet, - verification, - walletType: 'tss', - }); - isTransactionVerified.should.equal(true); + const { result, warnings } = await withVerificationWarnings(() => verifyTssFixture(txParams, txPrebuild, wallet)); + result.should.equal(true); + // The consolidation destination was checked against the base address, so nothing was + // reported as unverifiable. + warnings.should.have.length(0); }); it('should verify TSS transaction when txParams.type is consolidate', async function () { @@ -757,21 +806,17 @@ describe('ETH:', function () { }; const txPrebuild = { - txHex: '0x', + // A sweep of the address balance to the wallet base address. + txHex: await buildNativeTxHex(baseAddress, '1000000000000000000'), coin: 'hteth', walletId: 'fakeWalletId', }; - const verification = {}; - - const isTransactionVerified = await coin.verifyTransaction({ - txParams: txParams as any, - txPrebuild: txPrebuild as any, - wallet, - verification, - walletType: 'tss', - }); - isTransactionVerified.should.equal(true); + const { result, warnings } = await withVerificationWarnings(() => verifyTssFixture(txParams, txPrebuild, wallet)); + result.should.equal(true); + // The consolidation destination was checked against the base address, so nothing was + // reported as unverifiable. + warnings.should.have.length(0); }); it('should verify TSS transaction when txParams.prebuildTx has consolidateId', async function () { @@ -792,21 +837,17 @@ describe('ETH:', function () { }; const txPrebuild = { - txHex: '0x', + // A sweep of the address balance to the wallet base address. + txHex: await buildNativeTxHex(baseAddress, '1000000000000000000'), coin: 'hteth', walletId: 'fakeWalletId', }; - const verification = {}; - - const isTransactionVerified = await coin.verifyTransaction({ - txParams: txParams as any, - txPrebuild: txPrebuild as any, - wallet, - verification, - walletType: 'tss', - }); - isTransactionVerified.should.equal(true); + const { result, warnings } = await withVerificationWarnings(() => verifyTssFixture(txParams, txPrebuild, wallet)); + result.should.equal(true); + // The consolidation destination was checked against the base address, so nothing was + // reported as unverifiable. + warnings.should.have.length(0); }); it('should reject TSS transaction without recipients, consolidateId, or valid type', async function () { @@ -857,22 +898,19 @@ describe('ETH:', function () { walletPassphrase: 'fakeWalletPassphrase', }; + // An acceleration replaces an existing payment: the fee bump has no caller recipients to + // compare against, so the bytes only have to be a decodable EVM transaction. const txPrebuild = { - txHex: '0x', + txHex: await buildNativeTxHex(address2, '1000000000000000000'), coin: 'hteth', walletId: 'fakeWalletId', }; - const verification = {}; - - const isTransactionVerified = await coin.verifyTransaction({ - txParams: txParams as any, - txPrebuild: txPrebuild as any, - wallet, - verification, - walletType: 'tss', - }); - isTransactionVerified.should.equal(true); + const { result, warnings } = await withVerificationWarnings(() => verifyTssFixture(txParams, txPrebuild, wallet)); + result.should.equal(true); + // Accepted, but reported: the bytes were never compared against the original payment. + warnings.should.have.length(1); + warnings[0].should.match(/no fail-closed verifier for intent type 'acceleration'/); }); it('should verify TSS transaction with fillNonce type', async function () { @@ -890,22 +928,18 @@ describe('ETH:', function () { walletPassphrase: 'fakeWalletPassphrase', }; + // A nonce fill is a self-directed zero-value send: the recipient is the wallet base address. const txPrebuild = { - txHex: '0x', + txHex: await buildNativeTxHex(baseAddress, '0'), coin: 'hteth', walletId: 'fakeWalletId', }; - const verification = {}; - - const isTransactionVerified = await coin.verifyTransaction({ - txParams: txParams as any, - txPrebuild: txPrebuild as any, - wallet, - verification, - walletType: 'tss', - }); - isTransactionVerified.should.equal(true); + const { result, warnings } = await withVerificationWarnings(() => verifyTssFixture(txParams, txPrebuild, wallet)); + result.should.equal(true); + // Accepted, but reported: the verifier has no intent rules for a nonce fill. + warnings.should.have.length(1); + warnings[0].should.match(/no fail-closed verifier for intent type 'fillNonce'/); }); it('should verify TSS transaction with defiApprove type', async function () { @@ -1626,7 +1660,7 @@ describe('ETH:', function () { }); }; - ['tokenApproval', 'customTx', 'defiApprove'].forEach((type) => { + ['tokenApproval', 'defiApprove'].forEach((type) => { it(`should reject approve(wrapper, amount) on the underlying relabelled as ${type}`, async function () { await expectDefiRejection( verifyAs(type, defiCalldata('approve(address,uint256)', [wrapper, amount])), @@ -1635,64 +1669,57 @@ describe('ETH:', function () { }); }); - it('should reject a mutated-amount wrapper approve relabelled as customTx', async function () { + it('should reject a customTx intent outright — EVM has no verifiable shape for it', async function () { + // customTx is refused before any calldata check, so even a benign approve of another spender cannot pass. + await verifyAs('customTx', defiCalldata('approve(address,uint256)', [address2, amount])).should.be.rejectedWith( + /unsupported intent type for EVM TSS: customTx/ + ); + }); + + it('should reject a mutated-amount wrapper approve relabelled as tokenApproval', async function () { await expectDefiRejection( - verifyAs('customTx', defiCalldata('approve(address,uint256)', [wrapper, '999999999999'])), + verifyAs('tokenApproval', defiCalldata('approve(address,uint256)', [wrapper, '999999999999'])), /must be declared as a wrapApprove intent/ ); }); it('should reject a wrapper approve with trailing bytes (an ERC-20 ignores them)', async function () { await expectDefiRejection( - verifyAs('customTx', defiCalldata('approve(address,uint256)', [wrapper, amount]) + '00'), + verifyAs('tokenApproval', defiCalldata('approve(address,uint256)', [wrapper, amount]) + '00'), /must be declared as a wrapApprove intent/ ); }); it('should not affect approvals of the underlying to other spenders', async function () { - (await verifyAs('customTx', defiCalldata('approve(address,uint256)', [address2, amount]))).should.equal(true); + (await verifyAs('defiApprove', defiCalldata('approve(address,uint256)', [address2, amount]))).should.equal( + true + ); (await verifyAs('tokenApproval', defiCalldata('approve(address,uint256)', [address2, amount]))).should.equal( true ); }); - it('should not parse the tx at all when the calldata carries no approve selector', async function () { - const coin = bitgo.coin('hteth') as Hteth; - const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: address1 } }); - const verifyWith = async (data: string) => { - const txHex = await buildDefiTxHex(data, underlying); - // getTransactionBuilder is not part of the coin's public type, so spy through a minimal structural view. - const builderSpy = sinon.spy( - coin as unknown as { getTransactionBuilder: () => unknown }, - 'getTransactionBuilder' - ); - try { - await coin.verifyTransaction({ - txParams: { type: 'customTx', wallet } as any, - txPrebuild: { txHex, coin: 'hteth', walletId: 'w' } as any, - wallet, - verification: {}, - walletType: 'tss', - }); - return builderSpy.callCount; - } finally { - builderSpy.restore(); - } - }; - - (await verifyWith(defiCalldata('transfer(address,uint256)', [wrapper, amount]))).should.equal(0); - (await verifyWith(defiCalldata('approve(address,uint256)', [address2, amount]))).should.equal(1); + it('should not reject a transfer or an approve of another spender declared as tokenApproval', async function () { + // Only a wrapper-directed approve is forced through wrapApprove. A transfer, and an approve + // whose spender is not the wrapper, still reach signing; they are reported as unverified + // because tokenApproval has no intent rules here. + (await verifyAs('tokenApproval', defiCalldata('transfer(address,uint256)', [wrapper, amount]))).should.equal( + true + ); + (await verifyAs('tokenApproval', defiCalldata('approve(address,uint256)', [address2, amount]))).should.equal( + true + ); }); it('should leave other declared types untouched for txs that are not a wrapper approve', async function () { // Every type below skips the wrapApprove check and reaches the guard; none of these txs is an approve of a - // wrapper, so all must verify exactly as they did before the guard existed. + // wrapper, so all must still verify. customTx is deliberately absent: the verifier rejects that intent type + // outright, so it can no longer stand for an untouched type. const benign: Array<[string, string, string]> = [ - ['customTx', defiCalldata('transfer(address,uint256)', [address2, amount]), underlying], - ['customTx', defiCalldata('approve(address,uint256)', [address2, amount]), underlying], - ['customTx', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2], + ['tokenApproval', defiCalldata('transfer(address,uint256)', [address2, amount]), underlying], ['tokenApproval', defiCalldata('approve(address,uint256)', [address2, amount]), underlying], ['tokenApproval', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2], + ['defiApprove', defiCalldata('approve(address,uint256)', [address2, amount]), underlying], ['acceleration', defiCalldata('transfer(address,uint256)', [address2, amount]), address2], ['fillNonce', '0x', address2], ['enabletoken', defiCalldata('transfer(address,uint256)', [address2, amount]), address2], @@ -1703,9 +1730,11 @@ describe('ETH:', function () { }); it('should not affect transfers to the underlying or approves of the wrapper on other contracts', async function () { - (await verifyAs('customTx', defiCalldata('transfer(address,uint256)', [wrapper, amount]))).should.equal(true); + (await verifyAs('tokenApproval', defiCalldata('transfer(address,uint256)', [wrapper, amount]))).should.equal( + true + ); ( - await verifyAs('customTx', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2) + await verifyAs('tokenApproval', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2) ).should.equal(true); }); diff --git a/modules/sdk-coin-eth/test/unit/tssMinimalVerification.ts b/modules/sdk-coin-eth/test/unit/tssMinimalVerification.ts new file mode 100644 index 00000000000..5aa4414fd65 --- /dev/null +++ b/modules/sdk-coin-eth/test/unit/tssMinimalVerification.ts @@ -0,0 +1,928 @@ +import 'should'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; +import { BaseCoin as StaticsBaseCoin, coins, ContractAddressDefinedToken, EthereumNetwork } from '@bitgo/statics'; +import { + IBaseCoin, + PopulatedIntent, + resolveEffectiveTxParams, + TxIntentMismatchRecipientError, + TransactionParams, + TransactionType, + TxRequest, + Wallet, +} from '@bitgo/sdk-core'; + +import { Erc20Token, Eth, Hteth, optionalDeps, TransactionBuilder } from '../../src'; + +/** + * Minimal fail-closed EVM TSS payment + compatible consolidation verification + * (TOB-BITGOEDMPC-2 / INFOSEC-671 / CECHO-2460). + * + * Every case builds the real serialized transaction bytes the user's key share would sign and + * runs them through the coin's real verification entry point + * (`verifyTransaction({ walletType: 'tss' })` -> `verifyTssTransaction`). Nothing is stubbed: + * intent types, calldata, serialization formats and statics token contracts are the production + * ones. + * + * The behaviour this file pins: + * - a definite mismatch between the bytes and the user intent always throws, with no opt-in; + * - metadata that cannot be resolved locally only warns (it is not a mismatch) and never hides a + * definite mismatch that is independently provable; + * - `contractCall` / `customTx` are always rejected: EVM has no verifiable shape for them; + * - intent types without a handler warn instead of silently passing. + */ + +const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; +const secondRecipientAddress = '0x9c7e8ce6825bd48278b3ab59228ee26f8be7925b'; +const attackerAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; +const baseAddress = '0x1111111111111111111111111111111111111111'; +const otherTokenAddress = '0x2222222222222222222222222222222222222222'; +const MAX_UINT256 = ((1n << 256n) - 1n).toString(); + +/** Two distinct tokens registered in statics, resolved locally by name. */ +const tokenName = 'hteth:tusdc'; +const otherTokenName = 'hteth:tusdt'; + +const tokenContractOf = (name: string): string => { + const token = coins.get(name); + if (!(token instanceof ContractAddressDefinedToken)) { + throw new Error(`${name} has no contract address in statics`); + } + return String(token.contractAddress); +}; + +const tokenContractAddress = tokenContractOf(tokenName); +const otherTokenContractAddress = tokenContractOf(otherTokenName); + +/** Network batcher configured in statics for the chain under test. */ +const batcherAddress = String((coins.get('hteth').network as EthereumNetwork).batcherContractAddress ?? ''); +if (!/^0x[0-9a-fA-F]{40}$/.test(batcherAddress)) { + // A batch test built against a network with no batcher would silently exercise the + // unresolved-batcher warning instead of the comparison it names. + throw new Error(`hteth has no statics batcher contract address (got '${batcherAddress}')`); +} + +// --------------------------------------------------------------------------------------------- +// Calldata encoding — built with the ABI library the SDK ships, never hand-written hex. +// --------------------------------------------------------------------------------------------- + +const selector = (signature: string, types: string[]): string => + optionalDeps.ethAbi.methodID(signature, types).toString('hex'); + +const encode = (signature: string, types: string[], values: unknown[]): string => + `0x${selector(signature, types)}${optionalDeps.ethAbi.rawEncode(types, values).toString('hex')}`; + +const word = (value: bigint): string => value.toString(16).padStart(64, '0'); +const addrWord = (address: string): string => address.toLowerCase().replace(/^0x/, '').padStart(64, '0'); + +/** ERC-20 `transfer(address,uint256)` calldata. */ +const erc20Transfer = (to: string, amount: string): string => + `0x${selector('transfer', ['address', 'uint256'])}${addrWord(to)}${word(BigInt(amount))}`; + +/** ERC-20 `approve(address,uint256)` calldata — a known payment-wrong function. */ +const erc20Approve = (spender: string, amount: string): string => + `0x${selector('approve', ['address', 'uint256'])}${addrWord(spender)}${word(BigInt(amount))}`; + +/** Native batch `batch(address[],uint256[])` calldata. */ +const nativeBatch = (recipients: { address: string; amount: string }[]): string => + encode('batch', ['address[]', 'uint256[]'], [recipients.map((r) => r.address), recipients.map((r) => r.amount)]); + +/** Token batch `batchTransferFrom(address,address[],uint256[])` calldata. */ +const tokenBatch = (contract: string, recipients: { address: string; amount: string }[]): string => + encode( + 'batchTransferFrom', + ['address', 'address[]', 'uint256[]'], + [contract, recipients.map((r) => r.address), recipients.map((r) => r.amount)] + ); + +// --------------------------------------------------------------------------------------------- +// Serialized bytes +// --------------------------------------------------------------------------------------------- + +type Serialization = 'legacy' | 'eip1559'; + +/** + * Build the real transaction bytes for a send on `chain`. `legacy` builds a type-0 transaction, + * `eip1559` a type-2 one: the same intent must verify identically on both encodings. + */ +const buildTx = async ( + opts: { to: string; value?: string; data?: string }, + serialization: Serialization = 'legacy', + chain = 'hteth' +): Promise => { + const builder = new TransactionBuilder(coins.get(chain)); + builder.type(opts.data && opts.data !== '0x' ? TransactionType.ContractCall : TransactionType.SingleSigSend); + builder.fee( + serialization === 'eip1559' + ? { fee: '10', gasLimit: '100000', eip1559: { maxFeePerGas: '10', maxPriorityFeePerGas: '1' } } + : { fee: '10', gasLimit: '100000' } + ); + builder.counter(1); + builder.contract(opts.to); + if (opts.value !== undefined) { + builder.value(opts.value); + } + if (opts.data !== undefined) { + builder.data(opts.data); + } + return (await builder.build()).toBroadcastFormat(); +}; + +// --------------------------------------------------------------------------------------------- +// Verification harness +// --------------------------------------------------------------------------------------------- + +/** + * The verifier accepts the loose EVM signing shapes (`tokenData` on recipients, a stored + * `PopulatedIntent`, a caller `txParams` that predates `resolveEffectiveTxParams`). The public + * option type narrows some of those, so the call is made through its runtime signature; the + * object below is the exact one the TSS signing path passes. + */ +interface TssVerifyCall { + txParams: Record; + txPrebuild: { txHex: string; coin: string; walletId: string }; + wallet: Wallet; + verification: Record; + walletType: 'tss'; + intent?: PopulatedIntent; + callerTxParams?: Record; +} + +interface TssVerifier { + verifyTransaction(params: TssVerifyCall): Promise; +} + +const asVerifier = (coin: Eth): TssVerifier => coin as unknown as TssVerifier; + +let sharedClient: TestBitGoAPI | undefined; + +/** + * The decorated client registers every ERC-20 token constructor, which is expensive and has no + * per-test state, so one shared instance serves every harness. Only the coin and wallet under + * test are built per test. + */ +const makeClient = (): TestBitGoAPI => { + if (sharedClient) { + return sharedClient; + } + const client = TestBitGo.decorate(BitGoAPI, { env: 'test' }); + Erc20Token.createTokenConstructors().forEach(({ name, coinConstructor }) => + client.safeRegister(name, coinConstructor) + ); + client.safeRegister('eth', Eth.createInstance); + client.safeRegister('hteth', Hteth.createInstance); + client.initializeTestVars(); + sharedClient = client; + return client; +}; + +/** + * A coin instance for `staticsCoin`, used for chains this module does not register (kavaevm has + * no statics batcher contract, so it exercises the unresolved-batcher path with real bytes). + */ +const makeChainCoin = (client: TestBitGoAPI, staticsCoin: Readonly): Eth => + Eth.createInstance(client, staticsCoin) as unknown as Eth; + +interface VerifyOptions { + /** Stored tx request intent (pending-approval path). */ + intent?: Record; + /** Untouched caller params: `undefined` means the caller supplied none. */ + callerTxParams?: Record; + verification?: Record; +} + +interface Harness { + client: TestBitGoAPI; + verify: (txParams: Record, txHex: string, opts?: VerifyOptions) => Promise; + /** Mirrors the TSS signing path: the stored intent is folded into the effective txParams. */ + verifySigningPath: ( + callerTxParams: Record | undefined, + intent: Record, + txHex: string + ) => Promise; +} + +interface HarnessOptions { + chain?: string; + staticsCoin?: Readonly; + /** Wallet coinSpecific; the default harness carries the base address consolidations go to. */ + coinSpecific?: Record; +} + +const makeHarness = (options: HarnessOptions = {}): Harness => { + const chain = options.chain ?? 'hteth'; + const client = makeClient(); + const coin = options.staticsCoin + ? makeChainCoin(client, options.staticsCoin) + : (client.coin(chain) as unknown as Eth); + const wallet = new Wallet(client, coin as unknown as IBaseCoin, { + coinSpecific: options.coinSpecific ?? { baseAddress }, + }); + + const verify = (txParams: Record, txHex: string, opts: VerifyOptions = {}): Promise => + asVerifier(coin).verifyTransaction({ + txParams, + txPrebuild: { txHex, coin: chain, walletId: 'wallet-id' }, + wallet, + verification: opts.verification ?? {}, + walletType: 'tss', + ...(opts.intent ? { intent: opts.intent as unknown as PopulatedIntent } : {}), + ...(opts.callerTxParams ? { callerTxParams: opts.callerTxParams } : {}), + }); + + const verifySigningPath = ( + callerTxParams: Record | undefined, + intent: Record, + txHex: string + ): Promise => { + const txRequest = { txRequestId: 'signing-path', walletId: 'wallet-id', intent } as unknown as TxRequest; + // Caller params are the loose signing shapes (a `type` string, `tokenData` on recipients); + // resolveEffectiveTxParams only spreads them, so the structural cast is exact at runtime. + const effective = resolveEffectiveTxParams( + txRequest, + callerTxParams as unknown as TransactionParams | undefined, + chain + ); + return verify(effective as unknown as Record, txHex, { + intent, + ...(callerTxParams ? { callerTxParams } : {}), + }); + }; + + return { client, verify, verifySigningPath }; +}; + +interface CapturedRun { + result: boolean; + /** Every `console.warn` line the verifier emitted while running, pre-joined. */ + warnings: string[]; +} + +/** + * Run a verification while collecting the verifier's warning lines. Warnings are the only + * channel for cases the verifier cannot decide locally; they never change the return value. + */ +const captureWarnings = async (run: () => Promise): Promise => { + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]): void => { + warnings.push(args.map((arg) => String(arg)).join(' ')); + }; + try { + return { result: await run(), warnings }; + } finally { + console.warn = originalWarn; + } +}; + +const matching = (warnings: string[], reason: RegExp): string[] => warnings.filter((line) => reason.test(line)); + +/** + * The verifier refuses the bytes outright: it throws and never downgrades the case to a warning. + * Used where the failure is malformed bytes rather than a value mismatch with the intent. + */ +const expectFailClosed = async (run: () => Promise): Promise => { + let rejected = false; + const { warnings } = await captureWarnings(async () => { + try { + await run(); + } catch { + rejected = true; + } + return !rejected; + }); + rejected.should.equal(true, 'the verifier must reject these bytes'); + warnings.should.have.length(0); +}; + +// --------------------------------------------------------------------------------------------- + +describe('EVM TSS minimal payment verification (CECHO-2460)', function () { + this.timeout(120000); + + let harness: Harness; + + beforeEach(function () { + harness = makeHarness(); + }); + + describe('native payment', function () { + const intent = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + + it('verifies a matching native send with no warnings', async function () { + for (const serialization of ['legacy', 'eip1559'] as Serialization[]) { + const txHex = await buildTx({ to: recipientAddress, value: '1000' }, serialization); + const { result, warnings } = await captureWarnings(() => harness.verify(intent, txHex)); + result.should.equal(true); + warnings.should.have.length(0); + } + }); + + it('treats the stored spelling `payment` as the same intent', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '1000' }); + const { result, warnings } = await captureWarnings(() => + harness.verify({ type: 'payment', recipients: [{ address: recipientAddress, amount: '1000' }] }, txHex) + ); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('fails a redirected destination', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails an inflated value', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '9999' }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails payment calldata it cannot recognize as a payment shape', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '1000', data: '0xdeadbeef' }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + }); + + describe('ERC-20 token payment', function () { + const tokenRecipient = { + address: recipientAddress, + amount: '10000000', + tokenData: { tokenName }, + }; + const intent = { type: 'transfer', recipients: [tokenRecipient] }; + + it('verifies an exact transfer(recipient, amount) to the intended token contract', async function () { + for (const serialization of ['legacy', 'eip1559'] as Serialization[]) { + const txHex = await buildTx( + { to: tokenContractAddress, value: '0', data: erc20Transfer(recipientAddress, '10000000') }, + serialization + ); + const { result, warnings } = await captureWarnings(() => harness.verify(intent, txHex)); + result.should.equal(true); + warnings.should.have.length(0); + } + }); + + it('resolves the token from a bare top-level statics name', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Transfer(recipientAddress, '10000000'), + }); + const { result, warnings } = await captureWarnings(() => + harness.verify( + { type: 'transfer', tokenName, recipients: [{ address: recipientAddress, amount: '10000000' }] }, + txHex + ) + ); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('resolves a bare Ethereum mainnet `usdc` name to the trusted statics contract', async function () { + // Bare means no chain prefix: `usdc`, not `eth:usdc`. The contract the bytes are compared + // against must come from statics, so a wrong contract cannot be papered over. + const mainnetTokenName = 'usdc'; + const mainnetUsdc = coins.get(mainnetTokenName); + if (!(mainnetUsdc instanceof ContractAddressDefinedToken)) { + throw new Error(`statics has no token contract for '${mainnetTokenName}'`); + } + const usdcContractAddress = String(mainnetUsdc.contractAddress); + // Pin the canonical mainnet USDC contract: a statics change must not silently re-point this test. + usdcContractAddress.should.equal('0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'); + + const mainnetHarness = makeHarness({ chain: 'eth' }); + const mainnetIntent = { + type: 'transfer', + tokenName: mainnetTokenName, + recipients: [{ address: recipientAddress, amount: '10000000' }], + }; + const correctContractHex = await buildTx( + { to: usdcContractAddress, value: '0', data: erc20Transfer(recipientAddress, '10000000') }, + 'legacy', + 'eth' + ); + const { result, warnings } = await captureWarnings(() => + mainnetHarness.verify(mainnetIntent, correctContractHex) + ); + result.should.equal(true); + warnings.should.have.length(0); + + // A different, genuinely registered mainnet token must not satisfy a `usdc` intent. + const mainnetUsdt = coins.get('usdt'); + if (!(mainnetUsdt instanceof ContractAddressDefinedToken)) { + throw new Error("statics has no token contract for 'usdt'"); + } + const wrongContractHex = await buildTx( + { to: String(mainnetUsdt.contractAddress), value: '0', data: erc20Transfer(recipientAddress, '10000000') }, + 'legacy', + 'eth' + ); + await mainnetHarness + .verify(mainnetIntent, wrongContractHex) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('resolves the token the coin instance itself represents', async function () { + const tokenHarness = makeHarness(); + const tokenCoin = tokenHarness.client.coin(tokenName) as unknown as Eth; + const tokenWallet = new Wallet(tokenHarness.client, tokenCoin as unknown as IBaseCoin, { + coinSpecific: { baseAddress }, + }); + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Transfer(recipientAddress, '10000000'), + }); + const { result, warnings } = await captureWarnings(() => + asVerifier(tokenCoin).verifyTransaction({ + txParams: { type: 'transfer', recipients: [{ address: recipientAddress, amount: '10000000' }] }, + txPrebuild: { txHex, coin: tokenName, walletId: 'wallet-id' }, + wallet: tokenWallet, + verification: {}, + walletType: 'tss', + }) + ); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('fails a transfer sent to a different token contract', async function () { + const txHex = await buildTx({ + to: otherTokenContractAddress, + value: '0', + data: erc20Transfer(recipientAddress, '10000000'), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a wrong function under a token payment intent', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Approve(attackerAddress, MAX_UINT256), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a swapped inner recipient', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Transfer(attackerAddress, '10000000'), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a swapped inner amount', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Transfer(recipientAddress, '99999999'), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails native value riding along ERC-20 calldata', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '5', + data: erc20Transfer(recipientAddress, '10000000'), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails trailing calldata after a complete transfer(address,uint256) payload', async function () { + // An ERC-20 ignores trailing bytes; the verifier must not, or those bytes could carry a + // second call nobody verified. + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: `${erc20Transfer(recipientAddress, '10000000')}${'11'.repeat(32)}`, + }); + // The canonical payload alone verifies (the case above); the extra word must fail closed. + await expectFailClosed(() => harness.verify(intent, txHex)); + }); + }); + + describe('explicit authorized arbitrary calldata', function () { + const data = '0xdeadbeef'; + const intent = { type: 'transfer', recipients: [{ address: otherTokenAddress, amount: '7', data }] }; + + it('verifies an exact to / value / data match, including nonzero native value', async function () { + const txHex = await buildTx({ to: otherTokenAddress, value: '7', data }); + const { result, warnings } = await captureWarnings(() => harness.verify(intent, txHex)); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('fails a mismatched destination', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '7', data }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a mismatched value', async function () { + const txHex = await buildTx({ to: otherTokenAddress, value: '8', data }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails mismatched calldata', async function () { + const txHex = await buildTx({ to: otherTokenAddress, value: '7', data: '0xdeadbe00' }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + }); + + describe('batch payments', function () { + const batchRecipients = [ + { address: recipientAddress, amount: '10' }, + { address: secondRecipientAddress, amount: '20' }, + ]; + const intent = { type: 'transfer', recipients: batchRecipients }; + const tokenRecipients = batchRecipients.map((recipient) => ({ ...recipient, tokenData: { tokenName } })); + + it('verifies a native batch to the network batcher with value equal to the intent sum', async function () { + const txHex = await buildTx({ to: batcherAddress, value: '30', data: nativeBatch(batchRecipients) }); + const { result, warnings } = await captureWarnings(() => harness.verify(intent, txHex)); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('verifies a token batch without any top-level tokenName', async function () { + // Asset identity comes from the recipients' own token metadata; a top-level tokenName is + // not required and must not be invented from the transaction. + const txHex = await buildTx({ + to: batcherAddress, + value: '0', + data: tokenBatch(tokenContractAddress, batchRecipients), + }); + const { result, warnings } = await captureWarnings(() => + harness.verify({ type: 'transfer', recipients: tokenRecipients }, txHex) + ); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('fails a token batch sent through the native batch function', async function () { + const txHex = await buildTx({ to: batcherAddress, value: '0', data: nativeBatch(batchRecipients) }); + await harness + .verify({ type: 'transfer', recipients: tokenRecipients }, txHex) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a native batch sent through batchTransferFrom', async function () { + const txHex = await buildTx({ + to: batcherAddress, + value: '30', + data: tokenBatch(tokenContractAddress, batchRecipients), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a batch that is not sent to the network batcher', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '30', data: nativeBatch(batchRecipients) }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a batch whose calldata recipient differs from the intent', async function () { + const txHex = await buildTx({ + to: batcherAddress, + value: '30', + data: nativeBatch([ + { address: attackerAddress, amount: '10' }, + { address: secondRecipientAddress, amount: '20' }, + ]), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a batch whose calldata amount differs from the intent', async function () { + const txHex = await buildTx({ + to: batcherAddress, + value: '30', + data: nativeBatch([ + { address: recipientAddress, amount: '11' }, + { address: secondRecipientAddress, amount: '20' }, + ]), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a batch whose recipient count differs from the intent', async function () { + const txHex = await buildTx({ + to: batcherAddress, + value: '30', + data: nativeBatch([{ address: recipientAddress, amount: '30' }]), + }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a batch whose native value differs from the intent sum', async function () { + const txHex = await buildTx({ to: batcherAddress, value: '31', data: nativeBatch(batchRecipients) }); + await harness.verify(intent, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a token batch that moves a different token contract', async function () { + const txHex = await buildTx({ + to: batcherAddress, + value: '0', + data: tokenBatch(otherTokenContractAddress, batchRecipients), + }); + await harness + .verify({ type: 'transfer', recipients: tokenRecipients }, txHex) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('fails a token batch that carries native value', async function () { + const txHex = await buildTx({ + to: batcherAddress, + value: '5', + data: tokenBatch(tokenContractAddress, batchRecipients), + }); + await harness + .verify({ type: 'transfer', recipients: tokenRecipients }, txHex) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + }); + + describe('unresolved metadata never hides a definite mismatch', function () { + it('warns when the declared token cannot be resolved locally', async function () { + const txHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Transfer(recipientAddress, '1000'), + }); + const unresolved = { + type: 'transfer', + tokenName: 'hteth:not-a-registered-token', + recipients: [{ address: recipientAddress, amount: '1000' }], + }; + const { result, warnings } = await captureWarnings(() => harness.verify(unresolved, txHex)); + result.should.equal(true); + // The warning must name the identity it could not resolve: that is the missing fact. + matching(warnings, /hteth:not-a-registered-token/).should.not.be.empty(); + }); + + it('still fails the decoded recipient and amount on that transaction', async function () { + // The asset is unknown, but transfer(address,uint256) decodes exactly: the amount mismatch + // against the intent is provable without token metadata, so it must throw. + const amountHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Transfer(recipientAddress, '9999'), + }); + await harness + .verify( + { + type: 'transfer', + tokenName: 'hteth:not-a-registered-token', + recipients: [{ address: recipientAddress, amount: '1000' }], + }, + amountHex + ) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + + const recipientHex = await buildTx({ + to: tokenContractAddress, + value: '0', + data: erc20Transfer(attackerAddress, '1000'), + }); + await harness + .verify( + { + type: 'transfer', + tokenName: 'hteth:not-a-registered-token', + recipients: [{ address: recipientAddress, amount: '1000' }], + }, + recipientHex + ) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + }); + + describe('batch verification when the network has no batcher contract', function () { + const batchRecipients = [ + { address: recipientAddress, amount: '10' }, + { address: secondRecipientAddress, amount: '20' }, + ]; + let kavaHarness: Harness; + + beforeEach(function () { + // kavaevm has no `batcherContractAddress` in statics: the destination cannot be anchored, + // which is unresolved metadata, not a mismatch. + kavaHarness = makeHarness({ chain: 'kavaevm', staticsCoin: coins.get('kavaevm') }); + }); + + it('warns instead of failing a batch it cannot anchor to a batcher', async function () { + const txHex = await buildTx( + { to: attackerAddress, value: '30', data: nativeBatch(batchRecipients) }, + 'legacy', + 'kavaevm' + ); + const { result, warnings } = await captureWarnings(() => + kavaHarness.verify({ type: 'transfer', recipients: batchRecipients }, txHex) + ); + result.should.equal(true); + matching(warnings, /batcher/).should.not.be.empty(); + }); + + it('still fails a batch whose calldata amounts mismatch when the batcher is unresolved', async function () { + const txHex = await buildTx( + { + to: attackerAddress, + value: '30', + data: nativeBatch([ + { address: recipientAddress, amount: '11' }, + { address: secondRecipientAddress, amount: '20' }, + ]), + }, + 'legacy', + 'kavaevm' + ); + await kavaHarness + .verify({ type: 'transfer', recipients: batchRecipients }, txHex) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + }); + + describe('direct and stored intent provenance', function () { + const storedPaymentIntent = (address: string, value: string): Record => ({ + intentType: 'payment', + recipients: [{ address: { address }, amount: { value, symbol: 'hteth' } }], + }); + + it('verifies a stored payment intent when the caller declared nothing', async function () { + const txHex = await buildTx({ to: recipientAddress, value: '1000' }); + const { result, warnings } = await captureWarnings(() => + harness.verifySigningPath(undefined, storedPaymentIntent(recipientAddress, '1000'), txHex) + ); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('keeps the historic native checks throwing on the stored intent path', async function () { + const redirectHex = await buildTx({ to: attackerAddress, value: '1000' }); + await harness + .verifySigningPath(undefined, storedPaymentIntent(recipientAddress, '1000'), redirectHex) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('never fills a caller-declared payment that has no recipients from the stored intent', async function () { + // The caller asked for a payment but named nothing to verify against. The stored intent + // points at the attacker, so using it would let the server choose the destination. + const txHex = await buildTx({ to: attackerAddress, value: '9999' }); + await harness + .verifySigningPath({ type: 'transfer' }, storedPaymentIntent(attackerAddress, '9999'), txHex) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('uses the caller intent over the stored intent when both are present', async function () { + const callerIntent = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + const goodHex = await buildTx({ to: recipientAddress, value: '1000' }); + const storedIntent = storedPaymentIntent(attackerAddress, '1000'); + (await harness.verifySigningPath(callerIntent, storedIntent, goodHex)).should.equal(true); + + const attackerHex = await buildTx({ to: attackerAddress, value: '1000' }); + await harness + .verify(callerIntent, attackerHex, { callerTxParams: callerIntent, intent: storedIntent }) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + }); + + describe('intent types without a verifiable handler', function () { + it('always rejects contractCall and customTx', async function () { + const txHex = await buildTx({ to: otherTokenAddress, value: '0', data: '0xdeadbeef' }); + // Positive control: the same bytes are verifiable when the intent describes them exactly, + // so the rejection below comes from the declared type and not from the transaction. + const exactCall = { + type: 'transfer', + recipients: [{ address: otherTokenAddress, amount: '0', data: '0xdeadbeef' }], + }; + (await harness.verify(exactCall, txHex)).should.equal(true); + + for (const type of ['contractCall', 'customTx']) { + await harness + .verify({ type, recipients: [{ address: otherTokenAddress, amount: '0' }] }, txHex) + .should.be.rejectedWith(Error); + } + }); + + it('warns for a non-payment intent type that has no handler', async function () { + const txHex = await buildTx({ to: otherTokenAddress, value: '0', data: '0xdeadbeef' }); + for (const type of ['acceleration', 'bridgeFunds', 'enableToken']) { + const { result, warnings } = await captureWarnings(() => harness.verify({ type, recipients: [] }, txHex)); + result.should.equal(true, `${type} must not throw`); + // The warning names the intent type no verifier covers: that is the missing fact. + matching(warnings, new RegExp(type)).should.not.be.empty(); + } + }); + + it('rejects bytes it cannot decode before any handler runs', async function () { + const intent = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + // A placeholder payload has no transaction at all; a non-transaction payload has a valid + // looking prefix. Both fail closed instead of falling through to a warning. + for (const txHex of ['0x', '0xdeadbeef']) { + await expectFailClosed(() => harness.verify(intent, txHex)); + } + }); + }); + + describe('consolidation to the wallet base address', function () { + const consolidate = { type: 'consolidate', recipients: [] }; + + it('verifies a native consolidation that pays the base address', async function () { + const txHex = await buildTx({ to: baseAddress, value: '1000' }); + const { result, warnings } = await captureWarnings(() => harness.verify(consolidate, txHex)); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('verifies a token consolidation that pays the base address with no native value', async function () { + const txHex = await buildTx({ to: tokenContractAddress, value: '0', data: erc20Transfer(baseAddress, '5') }); + const { result, warnings } = await captureWarnings(() => harness.verify(consolidate, txHex)); + result.should.equal(true); + // No token identity is declared, so the base address, the calldata shape and the zero native + // value are verified while the token contract is not: that gap is reported, never claimed. + matching(warnings, /token contract cannot be checked/).should.not.be.empty(); + matching(warnings, /no fail-closed verifier/).should.be.empty(); + }); + + it('rejects a native consolidation paying someone other than the base address', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await harness + .verify(consolidate, txHex, { verification: { consolidationToBaseAddress: true } }) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('rejects token calldata that pays someone other than the base address', async function () { + const txHex = await buildTx({ to: tokenContractAddress, value: '0', data: erc20Transfer(attackerAddress, '5') }); + await harness + .verify(consolidate, txHex, { verification: { consolidationToBaseAddress: true } }) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('rejects the same native redirect on a plain consolidate intent', async function () { + // The wallet base address is locally known, so the expectation is established even without + // the caller-supplied verification flag: a definite mismatch must not need an opt-in. + const txHex = await buildTx({ to: attackerAddress, value: '1000' }); + await harness.verify(consolidate, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('rejects token calldata paying a different recipient on a plain consolidate intent', async function () { + const txHex = await buildTx({ to: tokenContractAddress, value: '0', data: erc20Transfer(attackerAddress, '5') }); + await harness.verify(consolidate, txHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('rejects native value riding along token-shaped consolidation calldata', async function () { + const txHex = await buildTx({ to: attackerAddress, value: '5000', data: erc20Transfer(baseAddress, '0') }); + await harness + .verify(consolidate, txHex, { verification: { consolidationToBaseAddress: true } }) + .should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('warns for a consolidation it cannot anchor when the wallet has no base address', async function () { + const anchorless = makeHarness({ coinSpecific: {} }); + const txHex = await buildTx({ to: baseAddress, value: '1000' }); + const { result, warnings } = await captureWarnings(() => anchorless.verify(consolidate, txHex)); + result.should.equal(true); + matching(warnings, /base address/).should.not.be.empty(); + }); + + it('warns for batch calldata on a consolidation instead of silently passing', async function () { + // This intent declares no asset, so the batch could belong to a token or specialised sweep: + // the unimplemented shape is reported, never claimed as verified. + const txHex = await buildTx({ + to: batcherAddress, + value: '30', + data: nativeBatch([ + { address: recipientAddress, amount: '10' }, + { address: secondRecipientAddress, amount: '20' }, + ]), + }); + const { result, warnings } = await captureWarnings(() => harness.verify(consolidate, txHex)); + result.should.equal(true); + matching(warnings, /batch/).should.not.be.empty(); + }); + }); + + describe('serialization formats', function () { + const intent = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + + it('runs the same checks on legacy and type-2 encodings', async function () { + const legacyHex = await buildTx({ to: attackerAddress, value: '1000' }, 'legacy'); + const eip1559Hex = await buildTx({ to: attackerAddress, value: '1000' }, 'eip1559'); + await harness.verify(intent, legacyHex).should.be.rejectedWith(TxIntentMismatchRecipientError); + await harness.verify(intent, eip1559Hex).should.be.rejectedWith(TxIntentMismatchRecipientError); + }); + + it('does not misclassify a supported encoding as an unverifiable format', async function () { + const legacyHex = await buildTx({ to: recipientAddress, value: '1000' }, 'legacy'); + const eip1559Hex = await buildTx({ to: recipientAddress, value: '1000' }, 'eip1559'); + for (const txHex of [legacyHex, eip1559Hex]) { + const { result, warnings } = await captureWarnings(() => harness.verify(intent, txHex)); + result.should.equal(true); + warnings.should.have.length(0); + } + }); + }); +}); diff --git a/modules/sdk-coin-evm/CHANGELOG.md b/modules/sdk-coin-evm/CHANGELOG.md index e8acaba6271..4d65fd9722d 100644 --- a/modules/sdk-coin-evm/CHANGELOG.md +++ b/modules/sdk-coin-evm/CHANGELOG.md @@ -3,6 +3,12 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## Unreleased + +### Bug Fixes + +* **sdk-coin-evm:** drop the legacy (non-EIP-1559) verifyTssTransaction bypass so every EVM TSS transaction runs the full intent verification. + # [1.17.0](https://github.com/BitGo/BitGoJS/compare/%40bitgo%2Fsdk-coin-evm%401.16.11...%40bitgo%2Fsdk-coin-evm%401.17.0) (2026-11-03) ### Features diff --git a/modules/sdk-coin-evm/src/evmCoin.ts b/modules/sdk-coin-evm/src/evmCoin.ts index 4da310e95cb..1013b8b4732 100644 --- a/modules/sdk-coin-evm/src/evmCoin.ts +++ b/modules/sdk-coin-evm/src/evmCoin.ts @@ -9,7 +9,6 @@ import { MPCAlgorithm, MultisigType, multisigTypes, - NO_RECIPIENT_TX_TYPES, } from '@bitgo/sdk-core'; import { BaseCoin as StaticsBaseCoin, CoinFeature, coins, CoinFamily } from '@bitgo/statics'; import { @@ -17,7 +16,6 @@ import { ERC721TransferBuilder, recoveryBlockchainExplorerQuery, TransactionBuilder as EthLikeTransactionBuilder, - VerifyEthTransactionOptions, } from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; import { isHtsEvmAddress, recovery_HBAREVM_BlockchainExplorerQuery, validateHederaAccountId } from './lib/utils'; @@ -87,47 +85,6 @@ export class EvmCoin extends AbstractEthLikeNewCoins { } } - /** @inheritDoc */ - async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const supportsEIP1559 = this.staticsCoin?.features?.includes(CoinFeature.EIP1559); - if (supportsEIP1559) { - return await super.verifyTssTransaction(params); - } else { - return await this.verifyLegacyTssTransaction(params); - } - } - - /** - * Verifies legacy (non-EIP-1559) TSS transactions with basic validation. - */ - private async verifyLegacyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const { txParams, txPrebuild, wallet } = params; - - // Basic validation for legacy transactions only - if ( - !txParams?.recipients && - !( - txParams.prebuildTx?.consolidateId || - txParams.stakingRequestId || - txParams.prebuildTx?.stakingRequestId || - (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) - ) - ) { - throw new Error(`missing txParams`); - } - - if (!wallet || !txPrebuild) { - throw new Error(`missing params`); - } - - if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) { - throw new Error(`tx cannot be both a batch and hop transaction`); - } - - // If validation passes, consider it verified - return true; - } - /** @inheritDoc */ isValidAddress(address: string, isAlternateAddress?: boolean): boolean { if (isAlternateAddress && this.getFamily() === CoinFamily.HBAREVM) { diff --git a/modules/sdk-coin-evm/test/unit/tssSigningVerification.ts b/modules/sdk-coin-evm/test/unit/tssSigningVerification.ts new file mode 100644 index 00000000000..081c0812d2b --- /dev/null +++ b/modules/sdk-coin-evm/test/unit/tssSigningVerification.ts @@ -0,0 +1,645 @@ +import 'should'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGo } from '@bitgo/sdk-test'; +import { coins, CoinFeature, EthereumNetwork } from '@bitgo/statics'; +import { + EcdsaMPCv2Utils, + EcdsaUtils, + InvalidTransactionError, + PopulatedIntent, + RequestTracer, + TransactionParams, + TransactionType, + TxIntentMismatchError, + TxIntentMismatchRecipientError, + TxRequest, + Wallet, +} from '@bitgo/sdk-core'; +import { getCommon, optionalDeps } from '@bitgo/abstract-eth'; + +import { EvmCoin } from '../../src/evmCoin'; +import { TransactionBuilder } from '../../src/lib'; + +/** + * CECHO-2460 / INFOSEC-671 — the user key share must not sign EVM bytes that were never compared + * with the intent the caller declared. + * + * These cases drive the real signing entries (`EcdsaUtils.signTxRequest` and + * `EcdsaMPCv2Utils.signTxRequest`), which run the real verifier + * (`AbstractEthLikeNewCoins.verifyTssTransaction`) on the real transaction hex before the signing + * round starts, and they only observe whether the signing round was reached. The verifier is not + * stubbed and the transaction bytes are built by the coin's own transaction builder. The signing + * round is replaced by a sentinel after verification, so `signingStarted` is decided by the + * production verifier alone. + * + * Three signing materials are covered on `tbaseeth` (Base Sepolia), a shared-SDK EVM chain that + * carries EIP-1559, so the coin's own `TransactionBuilder` and `getCommon` accept it: + * - the EIP-1559 typed envelope: `signableHex` is the unsigned typed payload (`0x02 || rlp([...])` + * without `v`, `r`, `s`); + * - the legacy type-0 envelope: the wallet platform can still build a `gasPrice` transaction for an + * EIP-1559 chain, and its `signableHex` is the EIP-155 preimage + * `rlp([nonce, gasPrice, gasLimit, to, value, data, chainId, 0, 0])`, which ethereumjs cannot + * parse (v = chainId), so only `serializedTxHex` describes what the user signs; + * - `tkavaevm`, a legacy-only shared-SDK chain that additionally forces the serialized-bytes route + * through `CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX`. + * + * Policy pinned by these cases: any definite mismatch between the bytes and the user intent throws + * in the single default mode — destination, amount, wrong or unexpected calldata, and a payment + * declared without recipients. Only a finding the verifier cannot decide locally (an intent type + * with no validator) warns and lets signing proceed. Whenever the verifier needs the serialized + * bytes, a missing `serializedTxHex` fails before the signing round instead of verifying a preimage + * the client cannot compare against anything. + */ + +const walletId = '62fe536a6b4cf70007acb48c0e7bb0b0'; +const baseAddress = '0x1111111111111111111111111111111111111111'; +const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; +const attackerAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; +const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; +const transferAmount = '1000'; +const approveAttackerMax = `0x095ea7b3${attackerAddress.slice(2).padStart(64, '0')}${'f'.repeat(64)}`; +// ERC-20 transfer(recipient, amount): a token function that a native payment must never carry. Its +// inner recipient and amount match the intent, so only the asset mismatch can reject it. +const erc20TransferToRecipient = `0xa9059cbb${recipientAddress.slice(2).padStart(64, '0')}${BigInt(transferAmount) + .toString(16) + .padStart(64, '0')}`; +// Minimal signing material: signRequestBase only needs pShare.i === 1, a backupNShare and the +// paillier modulus before it hands the unsigned bytes to verifyTransaction. +const prv = JSON.stringify({ pShare: { i: 1, n: 'paillier-modulus' }, backupNShare: {} }); +// MPCv2 takes the user key share as base64; the verifier runs before any share is used. +const mpcv2Prv = Buffer.from('mpcv2-user-key-share').toString('base64'); + +/** `auto` derives the envelope from the coin's statics features. */ +type Envelope = 'auto' | 'eip1559' | 'legacy'; +type TssImplementation = 'ecdsa' | 'mpcv2'; + +interface SigningMaterial { + /** Broadcast-format unsigned transaction, without the 0x prefix as the wallet platform sends it. */ + serializedTxHex: string; + /** The preimage the user key share signs: typed envelope or legacy EIP-155 RLP. */ + signableHex: string; +} + +interface TxShape { + to: string; + value: string; + data?: string; +} + +/** Build the two server-supplied hexes exactly as the wallet platform shapes them. */ +const buildSigningMaterial = async ( + chain: string, + tx: TxShape, + envelope: Envelope = 'auto' +): Promise => { + const staticsCoin = coins.get(chain); + const network = staticsCoin.network as EthereumNetwork; + const coinSupportsEip1559 = staticsCoin.features.includes(CoinFeature.EIP1559); + const isEip1559 = envelope === 'auto' ? coinSupportsEip1559 : envelope === 'eip1559'; + const fee = isEip1559 + ? { + fee: '30000000000', + gasLimit: '21000', + eip1559: { maxFeePerGas: '30000000000', maxPriorityFeePerGas: '1000000000' }, + } + : { fee: '30', gasPrice: '30', gasLimit: '21000' }; + + const builder = new TransactionBuilder(staticsCoin); + builder.type(tx.data ? TransactionType.ContractCall : TransactionType.SingleSigSend); + builder.fee(fee); + builder.counter(1); + builder.contract(tx.to); + if (tx.data) { + builder.data(tx.data); + } + builder.value(tx.value); + + const broadcast = (await builder.build()).toBroadcastFormat(); + const serializedTxHex = broadcast.startsWith('0x') ? broadcast.slice(2) : broadcast; + + const common = getCommon(network); + if (!coinSupportsEip1559) { + common.setHardfork(optionalDeps.EthCommon.Hardfork.Petersburg); + } + // getMessageToSign(false) is the signing preimage: the unsigned typed payload for EIP-1559, and + // [nonce, gasPrice, gasLimit, to, value, data, chainId, 0, 0] for a legacy EIP-155 transaction. + let signableHex: string; + if (isEip1559) { + const typedTx = optionalDeps.EthTx.FeeMarketEIP1559Transaction.fromSerializedTx( + Buffer.from(serializedTxHex, 'hex'), + { common } + ); + signableHex = typedTx.getMessageToSign(false).toString('hex'); + } else { + const legacyTx = optionalDeps.EthTx.Transaction.fromSerializedTx(Buffer.from(serializedTxHex, 'hex'), { common }); + signableHex = Buffer.from(optionalDeps.ethUtil.rlp.encode(legacyTx.getMessageToSign(false))).toString('hex'); + } + + return { serializedTxHex, signableHex }; +}; + +interface SignOutcome { + /** True only when execution got past verification and into the signing round. */ + signingStarted: boolean; + error?: Error; + /** Whatever the verifier reported through `console.warn` while deciding. */ + warnings: string[]; +} + +/** Runs the real ECDSA or MPCv2 signing path and reports whether the signing round began. */ +const sign = async (opts: { + chain: string; + material: SigningMaterial; + txParams?: TransactionParams; + intent?: PopulatedIntent; + implementation?: TssImplementation; + /** Drop the broadcast bytes, as a tampered or legacy server response can. */ + omitSerializedTxHex?: boolean; +}): Promise => { + const implementation = opts.implementation ?? 'ecdsa'; + const bitgo = TestBitGo.decorate(BitGoAPI, { env: 'test' }); + bitgo.safeRegister(opts.chain, EvmCoin.createInstance); + bitgo.initializeTestVars(); + + const coin = bitgo.coin(opts.chain); + const wallet = new Wallet(bitgo, coin, { id: walletId, coinSpecific: { baseAddress }, multisigType: 'tss' }); + const utils = + implementation === 'mpcv2' ? new EcdsaMPCv2Utils(bitgo, coin, wallet) : new EcdsaUtils(bitgo, coin, wallet); + + // Replace the first step after verification with a sentinel: reaching it means the verifier let + // these bytes through to the signing round. + let signingStarted = false; + if (implementation === 'mpcv2') { + const mpcv2 = utils as unknown as { + pickBitgoPubGpgKeyForSigning: () => Promise; + getMpcv2HashBuffer: () => Buffer; + }; + // The BitGo GPG key lookup is not the signing round; keep the suite offline. + mpcv2.pickBitgoPubGpgKeyForSigning = async () => ({}); + mpcv2.getMpcv2HashBuffer = () => { + signingStarted = true; + throw new Error('stop: signing round started'); + }; + } else { + const ecdsa = utils as unknown as { getEcdsaSigningChallenges: () => Promise }; + ecdsa.getEcdsaSigningChallenges = async () => { + signingStarted = true; + throw new Error('stop: signing round started'); + }; + } + + // Only the fields the signing path reads are populated; TxRequest carries many server fields. + const txRequest = { + txRequestId: 'signing-path', + apiVersion: 'full', + walletId, + intent: opts.intent, + transactions: [ + { + unsignedTx: { + derivationPath: 'm/0', + signableHex: opts.material.signableHex, + ...(opts.omitSerializedTxHex ? {} : { serializedTxHex: opts.material.serializedTxHex }), + }, + signatureShares: [], + }, + ], + } as unknown as TxRequest; + + const warnings: string[] = []; + const originalWarn = console.warn; + console.warn = (...args: unknown[]) => { + warnings.push(args.map((arg) => String(arg)).join(' ')); + }; + try { + await utils.signTxRequest({ + txRequest, + txParams: opts.txParams, + prv: implementation === 'mpcv2' ? mpcv2Prv : prv, + reqId: new RequestTracer(), + }); + } catch (e) { + return { signingStarted, error: e as Error, warnings }; + } finally { + console.warn = originalWarn; + } + return { signingStarted, warnings }; +}; + +/** + * The intent as the wallet platform stores it on a tx request. `symbol` is the chain symbol the + * platform labels the amount with; it may differ from the coin name by case (`TBASEETH`). + */ +const storedIntent = ( + chain: string, + intentType: string, + address: string, + value: string, + symbol: string = chain +): PopulatedIntent => ({ + intentType, + recipients: [{ address: { address }, amount: { value, symbol } }], +}); + +const callerTransfer = (address: string, amount: string): TransactionParams => ({ + type: 'transfer', + recipients: [{ address, amount }], +}); + +const expectRejectedWithoutSigning = (outcome: SignOutcome): void => { + outcome.signingStarted.should.equal(false, 'the signing round must never start'); + (outcome.error as Error).should.be.instanceOf(Error); + (outcome.error as Error).message.should.not.match(/signing round started/); +}; + +const expectSigned = (outcome: SignOutcome): void => { + outcome.signingStarted.should.equal(true, `expected the signing round to start, got: ${outcome.error?.message}`); + (outcome.error as Error).message.should.match(/signing round started/); +}; + +describe('EVM TSS signing path verifies the bytes before signing (CECHO-2460)', function () { + describe('EIP-1559 typed envelope (tbaseeth)', function () { + const chain = 'tbaseeth'; + + it('signs a caller-declared payment when the typed envelope matches that intent', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + // The fixture is realistic: the preimage is the unsigned typed payload, while the broadcast + // form the platform stores also carries the empty signature fields the preimage drops. + material.signableHex.should.not.equal(material.serializedTxHex); + + for (const type of ['transfer', 'payment']) { + const outcome = await sign({ + chain, + material, + txParams: { type, recipients: [{ address: recipientAddress, amount: transferAmount }] }, + }); + expectSigned(outcome); + } + }); + + it('never starts signing a payment the typed envelope redirects', async function () { + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + it('never starts signing a payment the typed envelope inflates', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: '2000' }); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/does not match the value given by client/); + }); + + it('never starts signing a caller-declared payment that declares no recipients', async function () { + // Without recipients there is nothing to verify the bytes against, and the stored intent is + // server data: it must not fill the gap and decide the destination the user signs. + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + txParams: { type: 'transfer' }, + intent: storedIntent(chain, 'payment', attackerAddress, transferAmount), + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/without recipients/); + }); + + it('signs a stored-intent payment the caller did not re-declare (pending approval)', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + intent: storedIntent(chain, 'payment', recipientAddress, transferAmount), + }); + expectSigned(outcome); + }); + + it('never starts signing stored-intent bytes that redirect the payment', async function () { + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + intent: storedIntent(chain, 'payment', recipientAddress, transferAmount), + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + }); + + it('ignores a stored intent that names another destination when the caller declared its own', async function () { + // The caller declared the payment; the stored intent is only server data and cannot reroute + // the caller's own declaration. + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + intent: storedIntent(chain, 'payment', attackerAddress, transferAmount), + }); + expectSigned(outcome); + }); + + it('never starts signing approve(attacker, MAX) labelled as a payment', async function () { + // Wrong and unexpected calldata under a payment intent is a definite mismatch: the user + // asked for a payment, the bytes grant an unlimited allowance. + const material = await buildSigningMaterial(chain, { + to: tokenContractAddress, + value: '0', + data: approveAttackerMax, + }); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchError); + }); + + it('never starts signing a stored contractCall intent it has no verifier for', async function () { + // `contractCall` / `customTx` carry arbitrary calldata with no intent shape to verify against, + // so they are refused outright instead of being signed unchecked. + const material = await buildSigningMaterial(chain, { + to: tokenContractAddress, + value: '0', + data: approveAttackerMax, + }); + const outcome = await sign({ + chain, + material, + intent: storedIntent(chain, 'contractCall', tokenContractAddress, '0'), + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).message.should.match(/unsupported intent type for EVM TSS: contractCall/); + }); + }); + + describe('legacy type-0 envelope on an EIP-1559 chain (tbaseeth)', function () { + const chain = 'tbaseeth'; + const envelope: Envelope = 'legacy'; + + it('signs a payment whose legacy serialized bytes match the intent', async function () { + // The wallet platform still builds gasPrice transactions for EIP-1559 chains. Their + // signableHex is the EIP-155 preimage (v = chainId), which ethereumjs cannot decode, so the + // verifier has to read the serialized transaction. + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }, envelope); + material.signableHex.should.not.equal(material.serializedTxHex); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectSigned(outcome); + }); + + it('never starts signing a legacy payment the serialized bytes redirect', async function () { + // A parse error instead of the recipient mismatch would mean the verifier decoded the + // EIP-155 preimage rather than the serialized transaction the user is signing. + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }, envelope); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + it('never starts signing legacy bytes the caller declared a different amount for', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: '2000' }, envelope); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/does not match the value given by client/); + }); + + it('never starts signing a preimage that was not derived from the verified legacy bytes', async function () { + const benign = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }, envelope); + const attacker = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }, envelope); + // Verify the benign bytes, then sign the attacker's preimage. + const material = { serializedTxHex: benign.serializedTxHex, signableHex: attacker.signableHex }; + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(InvalidTransactionError); + (outcome.error as Error).message.should.match(/signableHex is inconsistent with serializedTxHex/); + }); + }); + + describe('legacy type-0 envelope (tkavaevm)', function () { + const chain = 'tkavaevm'; + + it('signs a payment whose serialized bytes match the intent, and signs the matching preimage', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + // Legacy chains send two different hexes: the broadcast bytes and the EIP-155 preimage. + material.signableHex.should.not.equal( + material.serializedTxHex, + 'the preimage and the broadcast bytes must be distinct, as the platform sends them' + ); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectSigned(outcome); + }); + + it('never starts signing a payment redirected in the serialized bytes', async function () { + // A parse error instead of the recipient mismatch would mean the verifier decoded the + // EIP-155 preimage rather than the serialized transaction the user is signing. + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + it('never starts signing a preimage that was not derived from the verified serialized bytes', async function () { + const benign = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + const attacker = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + // Verify the benign bytes, then sign the attacker's preimage. + const material = { serializedTxHex: benign.serializedTxHex, signableHex: attacker.signableHex }; + const outcome = await sign({ chain, material, txParams: callerTransfer(recipientAddress, transferAmount) }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(InvalidTransactionError); + (outcome.error as Error).message.should.match(/signableHex is inconsistent with serializedTxHex/); + }); + + it('never starts signing a caller-declared payment that declares no recipients', async function () { + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + txParams: { type: 'transfer' }, + intent: storedIntent(chain, 'payment', attackerAddress, transferAmount), + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/without recipients/); + }); + + it('signs a stored-intent payment the caller did not re-declare (pending approval)', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + intent: storedIntent(chain, 'payment', recipientAddress, transferAmount), + }); + expectSigned(outcome); + }); + }); + + describe('stored native intent labelled with an upper-case chain symbol (tbaseeth)', function () { + const chain = 'tbaseeth'; + // The platform labels a native amount with the chain symbol, which it may upper-case. The + // recipient resolver must still recognise 'TBASEETH' as this chain's native coin, never as a + // token of that name: an unresolved asset would leave the payment unverifiable. + const nativeSymbol = 'TBASEETH'; + + it('signs a stored native payment whose amount symbol is upper-cased', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + intent: storedIntent(chain, 'payment', recipientAddress, transferAmount, nativeSymbol), + }); + expectSigned(outcome); + }); + + it('never starts signing a token function against a stored native payment', async function () { + // The intent declares a native amount and the bytes carry ERC-20 transfer(recipient, amount). + // Its inner recipient and amount match the intent, so the asset mismatch is the only finding. + const material = await buildSigningMaterial(chain, { + to: tokenContractAddress, + value: '0', + data: erc20TransferToRecipient, + }); + const outcome = await sign({ + chain, + material, + intent: storedIntent(chain, 'payment', recipientAddress, transferAmount, nativeSymbol), + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + }); + }); + + describe('missing serialized bytes', function () { + // A legacy preimage is not a parseable transaction, so the serialized broadcast bytes are the + // only thing that can be verified against intent. Without them the signing round must never + // start: verifying a preimage the client cannot compare against anything proves nothing. + const expectMissingSerializedRejection = (outcome: SignOutcome): void => { + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(InvalidTransactionError); + (outcome.error as Error).message.should.match(/serializedTxHex is required/); + }; + + it('never starts ECDSA signing when the legacy preimage has no serialized bytes', async function () { + const chain = 'tbaseeth'; + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }, 'legacy'); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + omitSerializedTxHex: true, + }); + expectMissingSerializedRejection(outcome); + }); + + it('never starts MPCv2 signing when the legacy preimage has no serialized bytes', async function () { + const chain = 'tbaseeth'; + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }, 'legacy'); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation: 'mpcv2', + omitSerializedTxHex: true, + }); + expectMissingSerializedRejection(outcome); + }); + + it('never starts signing when the coin forces the serialized route and the bytes are absent', async function () { + // `tkavaevm` declares CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, so the serialized route is + // taken from the coin's features rather than from the envelope. + const chain = 'tkavaevm'; + coins.get(chain).features.should.containEql(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX); + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + omitSerializedTxHex: true, + }); + expectMissingSerializedRejection(outcome); + }); + }); + + describe('MPCv2 signing path (tbaseeth)', function () { + const chain = 'tbaseeth'; + const implementation: TssImplementation = 'mpcv2'; + + it('signs a caller-declared payment when the typed envelope matches the intent', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectSigned(outcome); + }); + + it('never starts signing a typed payment the bytes redirect', async function () { + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + it('signs a legacy payment whose serialized bytes match the intent', async function () { + const material = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }, 'legacy'); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectSigned(outcome); + }); + + it('never starts signing a legacy payment the bytes redirect', async function () { + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }, 'legacy'); + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (outcome.error as Error).message.should.match(/destination address does not match/); + }); + + it('never starts signing a preimage that was not derived from the verified bytes', async function () { + const benign = await buildSigningMaterial(chain, { to: recipientAddress, value: transferAmount }, 'legacy'); + const attacker = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }, 'legacy'); + const material = { serializedTxHex: benign.serializedTxHex, signableHex: attacker.signableHex }; + const outcome = await sign({ + chain, + material, + txParams: callerTransfer(recipientAddress, transferAmount), + implementation, + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(InvalidTransactionError); + (outcome.error as Error).message.should.match(/signableHex is inconsistent with serializedTxHex/); + }); + + it('never starts signing a caller-declared payment that declares no recipients', async function () { + const material = await buildSigningMaterial(chain, { to: attackerAddress, value: transferAmount }); + const outcome = await sign({ + chain, + material, + txParams: { type: 'transfer' }, + intent: storedIntent(chain, 'payment', attackerAddress, transferAmount), + implementation, + }); + expectRejectedWithoutSigning(outcome); + (outcome.error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + }); + }); +}); diff --git a/modules/sdk-coin-flr/src/flr.ts b/modules/sdk-coin-flr/src/flr.ts index 908700b2913..bc21379fb42 100644 --- a/modules/sdk-coin-flr/src/flr.ts +++ b/modules/sdk-coin-flr/src/flr.ts @@ -36,6 +36,7 @@ import { UnsignedSweepTxMPCv2, RecoverOptions, OfflineVaultTxInfo, + VerifyEthTransactionOptions, } from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; import { FlrPLib } from '@bitgo/sdk-coin-flrp'; @@ -312,6 +313,109 @@ export class Flr extends AbstractEthLikeNewCoins { return true; } + /** + * Verify a TSS transaction against the intent the user approved. + * + * FLR cross-chain transactions are signed as Avalanche atomic transactions, which are not EVM + * transactions and cannot be decoded by the EVM intent verifier. This verifier recognizes the + * atomic cross-chain formats (`export`, `import`, `importtoc`) by decoding the bytes with the + * existing atomic parser: the codec prefix alone is not recognition, malformed bytes are + * rejected, and a transaction the parser understands keeps the behaviour it had before the EVM + * verifier became fail-closed. + * + * Recognizing the format is not verification: the export / import destination, amount and owner + * are not compared with the intent, so `true` from this method means "not rejected by this + * verifier", never "verified". The gap is reported as an explicit incomplete-verification + * warning. + * + * Residual risk this method does not close: on the MPCv2 path the atomic `serializedTxHex` + * verified here is not cryptographically bound to the pre-hashed `signableHex` that is actually + * signed (`assertSignableConsistency` runs only for TSS_VERIFY_USE_SERIALIZED_TX_HEX coins), so + * the bytes that get signed are not proven to be the bytes recognized above. + * + * Every other transaction keeps the parent verifier unchanged: non-atomic bytes are EVM bytes + * and are verified (or rejected) there, and atomic bytes declared as anything else stay on the + * parent path, which decides them exactly as it does for every other coin. + * + * @param {VerifyEthTransactionOptions} params + * @returns {Promise} + */ + async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { + // Avalanche atomic transactions carry the 0x0000 codec prefix, the same marker used by + // explainTransaction and isSignablePreHashed. The prefix is only a hint that decides whether + // the atomic parser gets asked; the decoded transaction decides whether the format is + // recognized. + const txHex = params.txPrebuild?.txHex; + const unprefixedHex = txHex?.startsWith('0x') ? txHex.slice(2) : txHex; + const declaredFormat = (Flr.declaredTssIntentType(params) ?? '').trim().toLowerCase(); + // Hop, missing-params and non-atomic cases keep the parent path unchanged; only a declared + // atomic cross-chain format backed by atomic-prefixed bytes is considered here. Imports carry + // no client recipients (the destination is the wallet itself, per NO_RECIPIENT_TX_TYPES); an + // export always names the P-chain recipients it pays, so a missing list keeps the parent's + // existing rejection. + if ( + !params.wallet || + params.txParams?.hop || + !unprefixedHex?.startsWith('0000') || + !['export', 'import', 'importtoc'].includes(declaredFormat) || + (declaredFormat === 'export' && !params.txParams?.recipients?.length) + ) { + return super.verifyTssTransaction(params); + } + + // The bytes must parse with the atomic parser: a bare 0x0000 prefix is not a recognized + // format. Nothing below compares the decoded destination, amount or owner with the intent. + this.assertDecodableAtomicTransaction(`0x${unprefixedHex}`); + + this.emitTssVerificationWarning( + `FLR ${declaredFormat} is an Avalanche atomic transaction: its bytes were recognized but not ` + + 'verified against the intent (destination, amount and ownership are not checked)', + declaredFormat + ); + return true; + } + + /** + * Decode atomic transaction bytes with the existing atomic parser. + * + * @param {string} txHex - the atomic transaction hex, with its codec prefix + * @returns {void} throws when the bytes are not a parseable Avalanche atomic transaction + * @private + */ + private assertDecodableAtomicTransaction(txHex: string): void { + try { + // `from` parses the bytes and selects the matching atomic transaction builder; it throws + // for anything the atomic codec cannot read, so no `build` (which would require a complete + // unsigned body) is needed to recognize the format. + this.getAtomicBuilder().from(txHex); + } catch (e) { + throw new Error( + `transaction bytes could not be decoded as an Avalanche atomic transaction: ${(e as Error).message}` + ); + } + } + + /** + * Resolve the intent type that speaks for the user, with the same precedence as the parent + * verifier: the caller's own params when it declared a type or recipients, otherwise the stored + * tx request intent. `txParams` cannot answer this because the linking path back-fills it from + * the stored intent, so `callerTxParams` (the untouched caller input) decides. + * + * @param {VerifyEthTransactionOptions} params + * @returns {string | undefined} the declared intent type + * @private + */ + private static declaredTssIntentType(params: VerifyEthTransactionOptions): string | undefined { + const callerParams = params.callerTxParams ?? (params.intent ? undefined : params.txParams); + const callerDeclaresIntent = !!(callerParams?.type || callerParams?.recipients?.length); + if (params.intent && !callerDeclaresIntent) { + // The parent prefers the back-filled `txParams.type` over `intent.intentType`; keep the same + // order so both verifiers always read the stored intent the same way. + return params.txParams?.type ?? params.intent.intentType; + } + return callerParams?.type; + } + /** * Check if an address is a FLR C-chain address (0x format) * @param {string} address - the address to check diff --git a/modules/sdk-coin-flr/test/resources.ts b/modules/sdk-coin-flr/test/resources.ts index 228293540ca..fe6ce2351b6 100644 --- a/modules/sdk-coin-flr/test/resources.ts +++ b/modules/sdk-coin-flr/test/resources.ts @@ -128,3 +128,11 @@ export const EXPORT_C_TEST_DATA = { // Private key for signing privKey: '14977929a4e00e4af1c33545240a6a5a08ca3034214618f6b04b72b80883be3a', }; + +// Import to C-chain (P -> C) Avalanche atomic transaction, unsigned. A real atomic import the +// parser understands, used to prove FLR keeps atomic imports signable (they carry no recipients). +export const IMPORT_IN_C_ATOMIC_TX = { + unsignedTxHex: + '0x0000000000000000007278db5c30bed04c05ce209179812850bbb3fe6d46d7eef3744d814c0da55524790000000000000000000000000000000000000000000000000000000000000000000000058781ab65cabfe88b9ee1d13d61182e07a4264b3774e351de3f0ebeb8314e92700000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000003473bc0000000020000000100000002ace9fd62aadb8b8825eb285edd311be25e8de543959616ead19f360fabc370560000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000003473bc0000000020000000100000002b489f3c616329c3d56a29c24702c348522e87e1ad0e23f02fbb405be599fbae30000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000003473bc0000000020000000100000002b9f3b7bb347a74a52688f98ab8d9c9a095420d63179d7c8030b2deb2dbe8d5500000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000003473bc0000000020000000100000002ba561801af08fa19dc3cd6767a328ed6d203ba82f27b8013220e7a26f1e3d1ac0000000158734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd000000050000000003473bc00000000200000001000000020000000117dbd11b9dd1c9be337353db7c14f9fb3662e5b500000000105f74b258734f94af871c3d131b56131b6fb7a0291eacadd261e69dfb42a9cdf6f7fddd0000000500000009000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000009000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000009000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000009000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf3665001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000009000000020000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007e918a5e8083ae4c9f2f0ed77055c24bf36650010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000b916ed7f', + to: '0x17Dbd11B9dD1c9bE337353db7C14f9fb3662E5B5', +}; diff --git a/modules/sdk-coin-flr/test/unit/flr.ts b/modules/sdk-coin-flr/test/unit/flr.ts index 602fd28d2be..72cdc319d5c 100644 --- a/modules/sdk-coin-flr/test/unit/flr.ts +++ b/modules/sdk-coin-flr/test/unit/flr.ts @@ -4,12 +4,17 @@ import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; import { coins } from '@bitgo/statics'; -import { Flr, Tflr } from '../../src/index'; -import { ExplainTransactionOptions } from '../../src/iface'; +import { Flr, Tflr, TransactionBuilder } from '../../src/index'; +import { ExplainTransactionOptions, TransactionPrebuild } from '../../src/iface'; import { UnsignedSweepTxMPCv2 } from '@bitgo/abstract-eth'; -import { mockDataUnsignedSweep, mockDataNonBitGoRecovery, EXPORT_C_TEST_DATA } from '../resources'; +import { + mockDataUnsignedSweep, + mockDataNonBitGoRecovery, + EXPORT_C_TEST_DATA, + IMPORT_IN_C_ATOMIC_TX, +} from '../resources'; import nock from 'nock'; -import { common, TransactionType, Wallet } from '@bitgo/sdk-core'; +import { common, PopulatedIntent, TransactionType, Wallet } from '@bitgo/sdk-core'; import { FeeMarketEIP1559Transaction } from '@ethereumjs/tx'; import { stripHexPrefix } from '@ethereumjs/util'; import * as sinon from 'sinon'; @@ -31,6 +36,19 @@ describe('flr', function () { let hopExportTxBitgoSignature; const address2 = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; + const tssBaseAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; + + // TSS verification only accepts bytes it can decode, so the flows below pass a real transaction + // instead of the '0x' placeholder that earlier fixtures used. + const buildTflrNativeTxHex = async (to: string, value: string): Promise => { + const txBuilder = new TransactionBuilder(coins.get('tflr')); + txBuilder.type(TransactionType.SingleSigSend); + txBuilder.fee({ fee: '10', gasLimit: '21000' }); + txBuilder.counter(1); + txBuilder.contract(to); + txBuilder.value(value); + return (await txBuilder.build()).toBroadcastFormat(); + }; const hopExportTx = EXPORT_C_TEST_DATA.fullsigntxHex; const hopExportTxId = '0x' + getTxHash(hopExportTx).toString('hex'); @@ -460,7 +478,7 @@ describe('flr', function () { // txPrebuild with consolidateId (set by server during consolidation build) const txPrebuild = { consolidateId: '68a7d5d0c66e74e216b97173bd558c6d', - txHex: '0x', + txHex: await buildTflrNativeTxHex(tssBaseAddress, '1000000000000000000'), coin: 'tflr', walletId: 'fakeWalletId', }; @@ -491,7 +509,7 @@ describe('flr', function () { }; const txPrebuild = { - txHex: '0x', + txHex: await buildTflrNativeTxHex(tssBaseAddress, '1000000000000000000'), coin: 'tflr', walletId: 'fakeWalletId', }; @@ -522,7 +540,8 @@ describe('flr', function () { }; const txPrebuild = { - txHex: '0x', + // acceleration carries no recipients; the bytes are still decoded by the verifier. + txHex: await buildTflrNativeTxHex(tssBaseAddress, '1'), coin: 'tflr', walletId: 'fakeWalletId', }; @@ -553,7 +572,7 @@ describe('flr', function () { }; const txPrebuild = { - txHex: '0x', + txHex: await buildTflrNativeTxHex(tssBaseAddress, '1'), coin: 'tflr', walletId: 'fakeWalletId', }; @@ -635,6 +654,210 @@ describe('flr', function () { }); }); + describe('TSS atomic cross-chain verification', function () { + // The fixture is a real Avalanche atomic transaction (0x0000 codec prefix). This verifier + // recognizes the encoding with the existing atomic parser only: it does not compare the + // destination, amount or owner with the intent, so an accepted transaction must always + // report that gap. + const exportPrebuild: TransactionPrebuild = { + txHex: EXPORT_C_TEST_DATA.unsignedTxHex, + coin: 'tflr', + buildParams: { recipients: [] }, + recipients: [], + nextContractSequenceId: 0, + gasPrice: 0, + gasLimit: 0, + isBatch: false, + }; + const pRecipient = { address: EXPORT_C_TEST_DATA.pMultisigAddress, amount: '1' }; + const atomicWarning = /\[tss verification\].*Avalanche atomic transaction.*not verified/; + + let exportWallet: Wallet; + + beforeEach(function () { + exportWallet = new Wallet(bitgo, tflrCoin, { + coinSpecific: { baseAddress: EXPORT_C_TEST_DATA.cHexAddress }, + }); + }); + + it('should keep a TSS atomic export signable and warn that it was not verified', async function () { + const warnStub = sinon.stub(console, 'warn'); + let verified: boolean; + try { + verified = await tflrCoin.verifyTransaction({ + txParams: { type: 'Export', recipients: [pRecipient] }, + txPrebuild: exportPrebuild, + wallet: exportWallet, + walletType: 'tss', + }); + } finally { + warnStub.restore(); + } + verified!.should.equal(true); + warnStub.callCount.should.equal(1); + String(warnStub.firstCall.args[0]).should.match(atomicWarning); + }); + + it('should recognize an export declared only by the stored pending-approval intent', async function () { + const warnStub = sinon.stub(console, 'warn'); + let verified: boolean; + try { + verified = await tflrCoin.verifyTransaction({ + // Pending approval: the caller declared nothing, resolveEffectiveTxParams back-filled + // txParams from the stored tx request intent. + txParams: { type: 'export', recipients: [pRecipient] }, + callerTxParams: undefined, + intent: { + intentType: 'export', + recipients: [ + { + address: { address: EXPORT_C_TEST_DATA.pMultisigAddress }, + amount: { value: '1', symbol: 'tflr' }, + }, + ], + } satisfies PopulatedIntent, + txPrebuild: exportPrebuild, + wallet: exportWallet, + walletType: 'tss', + }); + } finally { + warnStub.restore(); + } + verified!.should.equal(true); + String(warnStub.firstCall.args[0]).should.match(atomicWarning); + }); + + it('should keep atomic imports signable without recipients', async function () { + // Imports carry no client recipients: the destination is the wallet itself, which is why + // NO_RECIPIENT_TX_TYPES lists import / importtoc. The fixture is a real C-chain import the + // atomic parser understands, so it keeps the behaviour it had before the EVM verifier + // became fail-closed, with the incomplete-verification warning. + for (const type of ['ImportToC', 'importtoc', 'Import', 'import']) { + const warnStub = sinon.stub(console, 'warn'); + let verified: boolean; + try { + verified = await tflrCoin.verifyTransaction({ + txParams: { type }, + txPrebuild: { ...exportPrebuild, txHex: IMPORT_IN_C_ATOMIC_TX.unsignedTxHex }, + wallet: exportWallet, + walletType: 'tss', + }); + } finally { + warnStub.restore(); + } + verified!.should.equal(true); + String(warnStub.firstCall.args[0]).should.match(atomicWarning); + } + }); + + it('should not compare the export amount or destination with the intent', async function () { + // Ticket-scoped decision: this verifier recognizes the atomic encoding only. An amount + // that does not correspond to the export output is still accepted, and the warning is the + // contract with the caller: the export was not verified against its intent. + const warnStub = sinon.stub(console, 'warn'); + let verified: boolean; + try { + verified = await tflrCoin.verifyTransaction({ + txParams: { + type: 'Export', + recipients: [{ address: EXPORT_C_TEST_DATA.pAddresses[0], amount: '123456789' }], + }, + txPrebuild: exportPrebuild, + wallet: exportWallet, + walletType: 'tss', + }); + } finally { + warnStub.restore(); + } + verified!.should.equal(true); + String(warnStub.firstCall.args[0]).should.match(atomicWarning); + }); + + it('should keep rejecting atomic bytes declared as a payment', async function () { + // The caller declared a payment: atomic bytes are not an EVM payment, and the encoding + // must not carry them past the verifier that the declared intent selects. + const warnStub = sinon.stub(console, 'warn'); + try { + await tflrCoin + .verifyTransaction({ + txParams: { type: 'transfer', recipients: [{ address: tssBaseAddress, amount: '1' }] }, + txPrebuild: exportPrebuild, + wallet: exportWallet, + walletType: 'tss', + }) + .should.be.rejected(); + } finally { + warnStub.restore(); + } + warnStub.args + .map((args) => String(args[0])) + .filter((line) => atomicWarning.test(line)) + .should.have.length(0); + }); + + it('should keep the parent hop rejection for atomic bytes', async function () { + // Atomic encoding must not make the verifier skip the guards that already existed. + await tflrCoin + .verifyTransaction({ + txParams: { + type: 'Export', + hop: true, + recipients: [pRecipient, { address: EXPORT_C_TEST_DATA.pAddresses[0], amount: '1' }], + }, + txPrebuild: exportPrebuild, + wallet: exportWallet, + walletType: 'tss', + }) + .should.be.rejectedWith('tx cannot be both a batch and hop transaction'); + }); + + it('should keep rejecting an export declaration without recipients', async function () { + await tflrCoin + .verifyTransaction({ + txParams: { type: 'Export' }, + txPrebuild: exportPrebuild, + wallet: exportWallet, + walletType: 'tss', + }) + .should.be.rejectedWith('missing txParams'); + }); + + it('should reject malformed atomic bytes', async function () { + // The codec prefix alone is not recognition: bytes the atomic parser cannot read are not + // an atomic cross-chain transaction and must not be signed. + await tflrCoin + .verifyTransaction({ + txParams: { type: 'Export', recipients: [pRecipient] }, + txPrebuild: { ...exportPrebuild, txHex: '0x0000deadbeef' }, + wallet: exportWallet, + walletType: 'tss', + }) + .should.be.rejectedWith(/could not be decoded as an Avalanche atomic transaction/); + }); + + it('should leave EVM bytes declared as an export to the parent verifier', async function () { + const evmTxHex = await buildTflrNativeTxHex(tssBaseAddress, '1'); + const warnStub = sinon.stub(console, 'warn'); + try { + await tflrCoin.verifyTransaction({ + txParams: { type: 'export', recipients: [pRecipient] }, + txPrebuild: { ...exportPrebuild, txHex: evmTxHex }, + wallet: exportWallet, + walletType: 'tss', + }); + } catch { + // The parent owns the verdict for EVM bytes; this test only pins the routing. + } finally { + warnStub.restore(); + } + // Non-atomic bytes are never reported as a recognized atomic transaction. + warnStub.args + .map((args) => String(args[0])) + .filter((line) => atomicWarning.test(line)) + .should.have.length(0); + }); + }); + describe('Hop export tx verify', () => { const wallet = new Wallet(bitgo, tflrCoin, {}); const hopDestinationAddress = diff --git a/modules/sdk-coin-xdc/test/unit/xdc.ts b/modules/sdk-coin-xdc/test/unit/xdc.ts index f45db142b90..61e27d3df0e 100644 --- a/modules/sdk-coin-xdc/test/unit/xdc.ts +++ b/modules/sdk-coin-xdc/test/unit/xdc.ts @@ -134,7 +134,7 @@ describe('xdc', function () { const result = await coin.verifyTssTransaction({ txParams: { type: 'transfer', - recipients: [{ address: recipientAddress, amount: '10000000' }], + recipients: [{ address: recipientAddress, amount: '10000000', tokenAddress: tokenContractAddress }], } as any, txPrebuild: { txHex, coin: 'txdc', walletId: 'fakeWalletId' } as any, wallet, diff --git a/modules/sdk-coin-xdc/test/unit/xdcToken.ts b/modules/sdk-coin-xdc/test/unit/xdcToken.ts index 337b371094b..1de16b8ac3d 100644 --- a/modules/sdk-coin-xdc/test/unit/xdcToken.ts +++ b/modules/sdk-coin-xdc/test/unit/xdcToken.ts @@ -131,7 +131,9 @@ describe('XDC Token:', function () { describe('verifyTssTransaction', function () { const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; const wrongAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; - const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; + // The contract statics assigns to `txdc:tmt`; the token coin resolves its declared asset from + // this address, so the fixture must build the transfer against it. + const tokenContractAddress = '0xb283ec8dad644effc5c4c50bb7bb21442ac3c2db'; it('should accept an ERC-20 transfer where calldata matches declared recipient', async function () { const token = bitgo.coin('txdc:tmt') as XdcToken; diff --git a/modules/sdk-core/CHANGELOG.md b/modules/sdk-core/CHANGELOG.md index fae39f2b2e6..105825e1529 100644 --- a/modules/sdk-core/CHANGELOG.md +++ b/modules/sdk-core/CHANGELOG.md @@ -3,6 +3,12 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## Unreleased + +### Bug Fixes + +* **sdk-core:** pass the caller txParams and the stored tx request intent to verifyTransaction, bind signableHex to the verified serializedTxHex on the ECDSA signing path, route by the actual signable envelope so a legacy type-0 transaction on an EIP-1559-capable chain is verified from serializedTxHex, and keep a native intent amount (e.g. symbol 'ETH' on chain 'eth') out of tokenName by comparing the chain symbol case-insensitively. + # [40.5.0](https://github.com/BitGo/BitGoJS/compare/%40bitgo%2Fsdk-core%4040.4.0...%40bitgo%2Fsdk-core%4040.5.0) (2026-11-03) ### Bug Fixes diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index cb4ec7f61c7..c9411ddc27f 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -322,6 +322,26 @@ export interface VerifyTransactionOptions { verification?: VerificationOptions; reqId?: IRequestTracer; walletType?: 'onchain' | 'tss'; + /** + * The persisted TxRequest intent, set by the TSS signing path when verifying a + * pending-approval transaction. Verifiers use it as the source of truth for the + * intended `to` / `value` / `data` and for intent fields with no txParams equivalent + * (`tokenName`, `nonce`, `txid`, `consolidateId`, DeFi amounts). Never server-controlled + * beyond the tx request itself, which is the object the user approved. + */ + intent?: PopulatedIntent; + /** + * The `txParams` the caller of the signing API supplied, before `resolveEffectiveTxParams` + * back-filled `type` and `recipients` from the stored tx request intent. `undefined` when the + * caller supplied none (pending-approval signing of a stored tx request). + * + * `txParams` cannot answer "did the caller declare this?": the stored intent has already been + * merged into it. Verifiers that accept a stored `intent` use this to keep the caller's + * declared intent and the stored intent apart: a caller that declared `type` or `recipients` + * is the only source of truth, and the stored intent must not select the handler, add + * calldata, or name the asset. + */ + callerTxParams?: TransactionParams; } export interface SupplementGenerateWalletOptions { diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts index 0473a7ee397..d0649c6f23e 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts @@ -503,6 +503,8 @@ export interface PopulatedIntent extends PopulatedIntentBase, DefiIntentFields { feeToken?: string; hopParams?: HopParams; txid?: string; + /** Consolidation batch id; presence marks the tx as a consolidation to the wallet base address. */ + consolidateId?: string; receiveAddress?: string; custodianTransactionId?: string; custodianMessageId?: string; diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts index fece8b88d0c..0c3381d22b0 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts @@ -25,6 +25,7 @@ import { CustomMuDeltaShareGeneratingFunction, CustomPaillierModulusGetterFunction, CustomSShareGeneratingFunction, + PopulatedIntent, RequestType, TSSParams, TSSParamsForMessage, @@ -52,6 +53,7 @@ import { BaseEcdsaUtils } from './base'; import { EncryptionVersion, IRequestTracer } from '../../../../api'; import { resolveEffectiveTxParams } from '../recipientUtils'; import { shouldVerifyWithSerializedTxHex } from '../serializedTxHexVerify'; +import { isCoinWithSignableConsistency } from '../signableConsistency'; const encryptNShare = ECDSAMethods.encryptNShare; @@ -825,23 +827,38 @@ export class EcdsaUtils extends BaseEcdsaUtils { ); } - // For some coins, signableHex is not a parseable transaction. Pass - // serializedTxHex so verifyTransaction can decode the full tx bytes. - // Gated by CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX (ICP hash digests, - // BSC/XDC legacy EIP-155 RLP with v=chainId, etc.). - if (shouldVerifyWithSerializedTxHex(this.baseCoin)) { + // For some transactions signableHex is not a parseable transaction. Pass serializedTxHex + // so verifyTransaction can decode the full tx bytes. The routing looks at the actual + // envelope (a type-0 tx on an EIP-1559-capable chain carries the EIP-155 preimage) and + // only falls back to CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX for static cases + // (ICP hash digests, BSC/XDC legacy EIP-155 RLP with v=chainId, non-EIP-1559 chains). + const verifyWithSerializedTxHex = shouldVerifyWithSerializedTxHex(this.baseCoin, unsignedTx); + if (verifyWithSerializedTxHex) { + if (!unsignedTx.serializedTxHex) { + throw new InvalidTransactionError('serializedTxHex is required to verify this transaction before signing'); + } await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, - txParams: resolveEffectiveTxParams(txRequest, params.txParams), + txParams: resolveEffectiveTxParams(txRequest, params.txParams, this.baseCoin.getChain()), wallet: this.wallet, walletType: this.wallet.multisigType(), + intent: txRequest.intent as PopulatedIntent, + callerTxParams: params.txParams, }); + // The verifier above inspects serializedTxHex only, but the signing round below signs + // signableHex. Both are server-supplied, so bind them the same way the MPCv2 path does: + // otherwise verified bytes could be paired with a different signing preimage. + if (isCoinWithSignableConsistency(this.baseCoin)) { + this.baseCoin.assertSignableConsistency(unsignedTx.serializedTxHex, unsignedTx.signableHex); + } } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: resolveEffectiveTxParams(txRequest, params.txParams), + txParams: resolveEffectiveTxParams(txRequest, params.txParams, this.baseCoin.getChain()), wallet: this.wallet, walletType: this.wallet.multisigType(), + intent: txRequest.intent as PopulatedIntent, + callerTxParams: params.txParams, }); } signablePayload = Buffer.from(unsignedTx.signableHex, 'hex'); diff --git a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts index b942656b947..a60ec0c10e5 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -39,6 +39,7 @@ import { CustomMPCv2SigningRound1GeneratingFunction, CustomMPCv2SigningRound2GeneratingFunction, CustomMPCv2SigningRound3GeneratingFunction, + PopulatedIntent, RequestType, SignatureShareRecord, TSSParams, @@ -981,29 +982,44 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { ); } - // For some coins, signableHex is not a parseable transaction. Pass - // serializedTxHex so verifyTransaction can decode the full tx bytes. - // - CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX: ICP hash digests, - // BSC/XDC legacy EIP-155 RLP with v=chainId, etc. - // - Avalanche atomic (FLRP/FLR cross-chain): signableHex is SHA-256(txBody); - // serializedTxHex is the PVM/EVM atomic tx (codec prefix 0x0000). - // For other coins, signableHex IS the unsigned transaction (e.g. EIP-1559 RLP). - if (shouldVerifyWithSerializedTxHex(this.baseCoin) || shouldUsePreHashedSignable(this.baseCoin, unsignedTx)) { + // For some transactions signableHex is not a parseable transaction. Pass serializedTxHex + // so verifyTransaction can decode the full tx bytes: + // - Pre-hashed signable: Avalanche atomic cross-chain (FLRP/FLR) signableHex is + // SHA-256(txBody) and serializedTxHex is the PVM/EVM atomic tx (codec prefix 0x0000). + // That route stays as it is; the atomic payload must not be parsed as an EVM tx and + // there is no EVM signableHex to derive from it. + // - Everything else is routed by the actual envelope, not only coin features: an + // EIP-1559-capable chain can still carry a type-0 tx whose signableHex is the EIP-155 + // preimage. CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX remains the fallback for coins + // that cannot answer from the envelope (ICP hash digests, BSC/XDC legacy EIP-155 RLP). + const preHashedSignable = shouldUsePreHashedSignable(this.baseCoin, unsignedTx); + const verifyWithSerializedTxHex = + !preHashedSignable && shouldVerifyWithSerializedTxHex(this.baseCoin, unsignedTx); + if (verifyWithSerializedTxHex) { + if (!unsignedTx.serializedTxHex) { + throw new InvalidTransactionError('serializedTxHex is required to verify this transaction before signing'); + } + } + if (verifyWithSerializedTxHex || preHashedSignable) { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex, txInfo: unsignedTx.signableHex }, - txParams: resolveEffectiveTxParams(txRequest, params.txParams), + txParams: resolveEffectiveTxParams(txRequest, params.txParams, this.baseCoin.getChain()), wallet: this.wallet, walletType: this.wallet.multisigType(), + intent: txRequest.intent as PopulatedIntent, + callerTxParams: params.txParams, }); - if (shouldVerifyWithSerializedTxHex(this.baseCoin) && isCoinWithSignableConsistency(this.baseCoin)) { + if (verifyWithSerializedTxHex && isCoinWithSignableConsistency(this.baseCoin)) { this.baseCoin.assertSignableConsistency(unsignedTx.serializedTxHex, unsignedTx.signableHex); } } else { await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.signableHex }, - txParams: resolveEffectiveTxParams(txRequest, params.txParams), + txParams: resolveEffectiveTxParams(txRequest, params.txParams, this.baseCoin.getChain()), wallet: this.wallet, walletType: this.wallet.multisigType(), + intent: txRequest.intent as PopulatedIntent, + callerTxParams: params.txParams, }); } txOrMessageToSign = unsignedTx.signableHex; diff --git a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts index f33d44b9b05..9f314267c87 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -136,10 +136,14 @@ export function resolveEffectiveTxParams( const intentRecipients = (txRequest.intent as PopulatedIntent)?.recipients?.map((intentRecipient) => { // Prefer tokenData.tokenName; fall back to amount.symbol when chainName is // provided and differs from it. When absent, skip the symbol fallback. + // Native amounts are labelled with the chain symbol, which the platform may upper-case + // (e.g. 'ETH' for 'eth'), so the exclusion has to ignore case: otherwise a native payment + // would be reported as a token named 'ETH'. const { symbol } = intentRecipient.amount; + const isNativeSymbol = chainName !== undefined && !!symbol && symbol.toLowerCase() === chainName.toLowerCase(); const tokenName = intentRecipient.tokenData?.tokenName || - (chainName !== undefined && symbol && symbol !== chainName ? symbol : undefined); + (chainName !== undefined && symbol && !isNativeSymbol ? symbol : undefined); return { address: intentRecipient.address.address, amount: intentRecipient.amount.value, diff --git a/modules/sdk-core/src/bitgo/utils/tss/serializedTxHexVerify.ts b/modules/sdk-core/src/bitgo/utils/tss/serializedTxHexVerify.ts index 2a0509914c0..b2a51986048 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/serializedTxHexVerify.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/serializedTxHexVerify.ts @@ -1,12 +1,155 @@ import { CoinFeature } from '@bitgo/statics'; import { IBaseCoin } from '../../baseCoin'; +import type { SignableTransaction } from './baseTypes'; +import { isCoinWithPreHashedSignable } from './preHashedSignable'; +import { isCoinWithSignableConsistency } from './signableConsistency'; + +/** The two server-supplied hexes of one unsigned transaction. */ +export interface TssSignableEnvelope { + signableHex?: string; + serializedTxHex?: string; +} + +/** A legacy EVM transaction is exactly 9 RLP fields: nonce, gasPrice, gasLimit, to, value, data, v, r, s. */ +const LEGACY_TX_FIELD_COUNT = 9; + +function toBuffer(hex: string): Buffer | undefined { + const stripped = hex.startsWith('0x') ? hex.slice(2) : hex; + if (stripped.length === 0 || stripped.length % 2 !== 0 || !/^[0-9a-fA-F]+$/.test(stripped)) { + return undefined; + } + return Buffer.from(stripped, 'hex'); +} + +/** + * Returns the payload of a single RLP list that consumes the whole buffer, or undefined when the + * bytes are not exactly one well-formed RLP list. + */ +function rlpListPayload(bytes: Buffer): Buffer | undefined { + if (bytes.length < 2) { + return undefined; + } + const prefix = bytes[0]; + if (prefix < 0xc0) { + return undefined; // not a list + } + if (prefix <= 0xf7) { + const length = prefix - 0xc0; + return 1 + length === bytes.length ? bytes.subarray(1) : undefined; + } + const lengthOfLength = prefix - 0xf7; + if (1 + lengthOfLength >= bytes.length) { + return undefined; + } + let length = 0; + for (let i = 0; i < lengthOfLength; i++) { + length = length * 256 + bytes[1 + i]; + } + return 1 + lengthOfLength + length === bytes.length ? bytes.subarray(1 + lengthOfLength) : undefined; +} + +/** Counts the top-level items of an RLP list payload, or undefined when it is malformed. */ +function countRlpListItems(payload: Buffer): number | undefined { + let offset = 0; + let items = 0; + while (offset < payload.length) { + const prefix = payload[offset]; + // A single byte below 0x80 is its own data with no header; every other item has a header. + let headerLength = 0; + let dataLength = 1; + if (prefix >= 0x80) { + if (prefix <= 0xb7) { + headerLength = 1; + dataLength = prefix - 0x80; + } else if (prefix <= 0xbf) { + const lengthOfLength = prefix - 0xb7; + if (offset + 1 + lengthOfLength > payload.length) { + return undefined; + } + headerLength = 1 + lengthOfLength; + dataLength = 0; + for (let i = 0; i < lengthOfLength; i++) { + dataLength = dataLength * 256 + payload[offset + 1 + i]; + } + } else if (prefix <= 0xf7) { + headerLength = 1; + dataLength = prefix - 0xc0; + } else { + const lengthOfLength = prefix - 0xf7; + if (offset + 1 + lengthOfLength > payload.length) { + return undefined; + } + headerLength = 1 + lengthOfLength; + dataLength = 0; + for (let i = 0; i < lengthOfLength; i++) { + dataLength = dataLength * 256 + payload[offset + 1 + i]; + } + } + } + offset += headerLength + dataLength; + if (offset > payload.length) { + return undefined; + } + items += 1; + } + return items; +} + +/** + * True when `signableHex` is a legacy (type-0) EVM envelope: exactly one 9-field RLP list. + * + * A legacy transaction is signed over the EIP-155 preimage + * `RLP([nonce, gasPrice, gasLimit, to, value, data, chainId, 0, 0])`, which ethereumjs refuses to + * parse as a transaction ("Incompatible EIP155-based V"). An unsigned legacy broadcast tx has the + * same 9-field shape, so both are verified through serializedTxHex. Typed envelopes + * (0x01/0x02/0x03/0x04) and non-RLP signables (hashes, native/UTXO formats) do not match. + */ +export function isLegacyEvmSignableHex(signableHex: string): boolean { + const bytes = toBuffer(signableHex); + if (!bytes) { + return false; + } + const payload = rlpListPayload(bytes); + return payload !== undefined && countRlpListItems(payload) === LEGACY_TX_FIELD_COUNT; +} /** - * Returns true when TSS verifyTransaction should decode serializedTxHex instead of - * signableHex (e.g. ICP hash digests, or legacy EIP-155 RLP that fails ethereumjs - * fromSerializedTx). Controlled via CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX. + * Returns true when TSS verifyTransaction should decode serializedTxHex instead of signableHex. + * + * CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX wins (ICP hash digests, BSC/XDC legacy EIP-155 RLP + * where signableHex is never a parseable transaction, chains that never activated EIP-1559). + * Otherwise the decision follows the actual envelope: a type-0 transaction on an EIP-1559-capable + * chain carries the EIP-155 preimage in signableHex and must be verified through serializedTxHex, + * while a type-2 signableHex is already the unsigned transaction and is verified directly. + * + * Only coins that can bind the two hexes (assertSignableConsistency) are considered for the + * envelope route, and pre-hashed signables (Avalanche atomic) keep their own route. Everything + * else keeps the upstream behaviour. + * + * The legacy decision is made from signableHex alone: a legacy preimage must never fall back to + * the signable route just because serializedTxHex is absent. Callers on the serialized route must + * reject a missing serializedTxHex instead of verifying bytes they do not have. */ -export function shouldVerifyWithSerializedTxHex(coin: IBaseCoin): boolean { - return coin.getConfig().features?.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX) ?? false; +export function shouldVerifyWithSerializedTxHex(coin: IBaseCoin, unsignedTx?: TssSignableEnvelope): boolean { + if (coin.getConfig().features?.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX)) { + return true; + } + const { signableHex, serializedTxHex } = unsignedTx ?? {}; + if (!signableHex) { + return false; + } + // Pre-hashed signables are checked before the envelope so a preimage-shaped hex never pulls an + // Avalanche atomic payload into the EVM route. + if ( + isCoinWithPreHashedSignable(coin) && + serializedTxHex && + coin.isSignablePreHashed(unsignedTx as SignableTransaction) + ) { + return false; + } + if (!isCoinWithSignableConsistency(coin)) { + return false; + } + return isLegacyEvmSignableHex(signableHex); } diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts index a72732aeeb3..a4861a42de4 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -636,6 +636,7 @@ describe('ECDSA MPC v2', async () => { verifyTransaction: sinon.stub().resolves(true), getMPCAlgorithm: sinon.stub().returns('ecdsa'), getConfig: sinon.stub().returns({ family: 'flrp' }), + getChain: sinon.stub().returns('flrp'), isSignablePreHashed: (unsignedTx: { serializedTxHex?: string }) => unsignedTx.serializedTxHex?.startsWith('0000') ?? false, } as unknown as IBaseCoin; @@ -707,6 +708,7 @@ describe('ECDSA MPC v2', async () => { verifyTransaction: sinon.stub().resolves(true), getMPCAlgorithm: sinon.stub().returns('ecdsa'), getConfig: sinon.stub().returns({ family: 'flr' }), + getChain: sinon.stub().returns('flr'), } as unknown as IBaseCoin; const mockWallet = { @@ -781,6 +783,7 @@ describe('ECDSA MPC v2', async () => { verifyTransaction: verifyTransactionSpy, getMPCAlgorithm: sinon.stub().returns('ecdsa'), getConfig: sinon.stub().returns({ family: 'hteth' }), + getChain: sinon.stub().returns('hteth'), } as unknown as IBaseCoin; const mockWallet = { diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts index bb6ffbf5680..c9ed12c254e 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -413,6 +413,42 @@ describe('recipientUtils', function () { assert.strictEqual(result.recipients?.[0].tokenName, undefined); }); + it('does NOT set tokenName when a native symbol differs from chainName only by case', function () { + // The platform stores native amounts with the upper-case chain symbol ('ETH' on 'eth'). + // That must stay a native payment, not become a token named 'ETH'. + const txRequest = makeTxRequest({ + intent: { + intentType: 'payment', + recipients: [ + { + address: { address: '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be' }, + amount: { value: '1000', symbol: 'ETH' }, + }, + ], + } as PopulatedIntent, + }); + const result = resolveEffectiveTxParams(txRequest, {}, 'eth'); + assert.strictEqual(result.recipients?.length, 1); + assert.strictEqual(result.recipients?.[0].address, '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'); + assert.strictEqual(result.recipients?.[0].tokenName, undefined); + }); + + it('still sets tokenName when the symbol differs from chainName beyond case', function () { + const txRequest = makeTxRequest({ + intent: { + intentType: 'payment', + recipients: [ + { + address: { address: '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be' }, + amount: { value: '1000', symbol: 'ETH:USDC' }, + }, + ], + } as PopulatedIntent, + }); + const result = resolveEffectiveTxParams(txRequest, {}, 'eth'); + assert.strictEqual(result.recipients?.[0].tokenName, 'ETH:USDC'); + }); + it('prefers tokenData.tokenName over amount.symbol (uses distinct values to verify)', function () { const txRequest = makeTxRequest({ intent: { diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/serializedTxHexVerify.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/serializedTxHexVerify.ts new file mode 100644 index 00000000000..9e03fccdb00 --- /dev/null +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/serializedTxHexVerify.ts @@ -0,0 +1,117 @@ +import * as assert from 'assert'; +import { CoinFeature } from '@bitgo/statics'; +import { IBaseCoin } from '../../../../../src'; +import { + isLegacyEvmSignableHex, + shouldVerifyWithSerializedTxHex, +} from '../../../../../src/bitgo/utils/tss/serializedTxHexVerify'; + +describe('serializedTxHexVerify', function () { + // 9-field legacy envelope: RLP([nonce, gasPrice, gasLimit, to, value, data, chainId, 0, 0]). + // This is the EIP-155 signing preimage the platform sends as signableHex for a type-0 send, + // even on chains that also support EIP-1559. + // 0xe4 = short RLP list of 0x24 = 36 payload bytes: 1 + 1 + 3 + 21 + 3 + 1 + 4 + 1 + 1. + const legacyPreimageHex = + 'e4' + '01' + '1e' + '825208' + '94' + '11'.repeat(20) + '8203e8' + '80' + '83088bb0' + '8080'; + + const makeCoin = (opts: { + features?: CoinFeature[]; + signablePreHashed?: boolean; + withConsistency?: boolean; + }): IBaseCoin => { + return { + getConfig: () => ({ features: opts.features ?? [] }), + ...(opts.signablePreHashed !== undefined && { + isSignablePreHashed: () => opts.signablePreHashed, + }), + ...(opts.withConsistency !== false && { assertSignableConsistency: () => undefined }), + } as unknown as IBaseCoin; + }; + + describe('isLegacyEvmSignableHex', function () { + it('detects a 9-field legacy envelope, with or without 0x', function () { + assert.strictEqual(isLegacyEvmSignableHex(legacyPreimageHex), true); + assert.strictEqual(isLegacyEvmSignableHex('0x' + legacyPreimageHex), true); + }); + + it('does not treat a typed (EIP-1559) envelope as legacy', function () { + // 0x02 || RLP([...]) — the unsigned type-2 envelope the platform sends as signableHex. + assert.strictEqual(isLegacyEvmSignableHex('02' + legacyPreimageHex), false); + }); + + it('does not treat hashes or non-RLP signables as legacy', function () { + assert.strictEqual(isLegacyEvmSignableHex('11'.repeat(32)), false, '32-byte digest'); + assert.strictEqual(isLegacyEvmSignableHex('0000' + '11'.repeat(30)), false, 'Avalanche codec payload'); + assert.strictEqual(isLegacyEvmSignableHex('not-hex'), false); + }); + + it('does not treat an RLP list with the wrong field count as legacy', function () { + // RLP list of two fields: 0xc2, 0x01, 0x02. + assert.strictEqual(isLegacyEvmSignableHex('c20102'), false); + }); + }); + + describe('shouldVerifyWithSerializedTxHex', function () { + it('routes a type-0 envelope on an EIP-1559-capable coin through serializedTxHex', function () { + const coin = makeCoin({ features: [CoinFeature.EIP1559] }); + assert.strictEqual( + shouldVerifyWithSerializedTxHex(coin, { signableHex: legacyPreimageHex, serializedTxHex: legacyPreimageHex }), + true + ); + }); + + it('verifies a type-2 signableHex directly, without decoding serializedTxHex', function () { + const coin = makeCoin({ features: [CoinFeature.EIP1559] }); + assert.strictEqual( + shouldVerifyWithSerializedTxHex(coin, { + signableHex: '02' + legacyPreimageHex, + serializedTxHex: '02' + legacyPreimageHex + 'c08080', + }), + false + ); + }); + + it('honours CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX regardless of envelope', function () { + const coin = makeCoin({ features: [CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX] }); + assert.strictEqual( + shouldVerifyWithSerializedTxHex(coin, { signableHex: '11'.repeat(32), serializedTxHex: '22'.repeat(32) }), + true + ); + }); + + it('keeps the pre-hashed route for Avalanche atomic signables', function () { + // A legacy-shaped signable would otherwise route to serializedTxHex, which for atomic + // exports is an Avalanche codec payload that must not be parsed as an EVM transaction. + const coin = makeCoin({ signablePreHashed: true }); + assert.strictEqual( + shouldVerifyWithSerializedTxHex(coin, { + signableHex: legacyPreimageHex, + serializedTxHex: '0000' + '11'.repeat(30), + }), + false + ); + }); + + it('does not route coins that cannot bind the two hexes', function () { + const coin = makeCoin({ withConsistency: false }); + assert.strictEqual( + shouldVerifyWithSerializedTxHex(coin, { signableHex: legacyPreimageHex, serializedTxHex: legacyPreimageHex }), + false + ); + }); + + it('detects a legacy envelope from signableHex alone, even without serializedTxHex', function () { + // The route must not silently fall back to an unparseable preimage just because the + // serialized bytes are missing: the signing paths reject that case explicitly. + const coin = makeCoin({ features: [CoinFeature.EIP1559] }); + assert.strictEqual(shouldVerifyWithSerializedTxHex(coin, { signableHex: legacyPreimageHex }), true); + }); + + it('does not route without a signableHex or with an empty one', function () { + const coin = makeCoin({ features: [CoinFeature.EIP1559] }); + assert.strictEqual(shouldVerifyWithSerializedTxHex(coin, { serializedTxHex: 'ab' }), false); + assert.strictEqual(shouldVerifyWithSerializedTxHex(coin, { signableHex: '', serializedTxHex: 'ab' }), false); + assert.strictEqual(shouldVerifyWithSerializedTxHex(coin), false); + }); + }); +}); diff --git a/modules/statics/CHANGELOG.md b/modules/statics/CHANGELOG.md index ca530236e44..2e44f6a6680 100644 --- a/modules/statics/CHANGELOG.md +++ b/modules/statics/CHANGELOG.md @@ -3,6 +3,13 @@ All notable changes to this project will be documented in this file. See [Conventional Commits](https://conventionalcommits.org) for commit guidelines. +## Unreleased + +### Bug Fixes + +* **statics:** add the verified Katana mainnet and testnet batcher contract address so batch prebuilds are checked against the deployed batcher. +* **statics:** add TSS_VERIFY_USE_SERIALIZED_TX_HEX to kavaevm, tkavaevm, prividiumeth and tprividiumeth, whose signableHex is the EIP-155 preimage. + # [59.28.0](https://github.com/BitGo/BitGoJS/compare/%40bitgo%2Fstatics%4059.27.0...%40bitgo%2Fstatics%4059.28.0) (2026-11-03) ### Bug Fixes diff --git a/modules/statics/src/allCoinsAndTokens.ts b/modules/statics/src/allCoinsAndTokens.ts index 54c495c9f14..f49e9856ba3 100644 --- a/modules/statics/src/allCoinsAndTokens.ts +++ b/modules/statics/src/allCoinsAndTokens.ts @@ -2170,6 +2170,7 @@ export const allCoinsAndTokens = [ CoinFeature.EVM_COMPATIBLE_WP, CoinFeature.EVM_NON_BITGO_RECOVERY, CoinFeature.EVM_UNSIGNED_SWEEP_RECOVERY, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ] ), account( @@ -2189,6 +2190,7 @@ export const allCoinsAndTokens = [ CoinFeature.EVM_COMPATIBLE_WP, CoinFeature.EVM_NON_BITGO_RECOVERY, CoinFeature.EVM_UNSIGNED_SWEEP_RECOVERY, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ] ), account( @@ -2878,6 +2880,7 @@ export const allCoinsAndTokens = [ CoinFeature.EVM_COMPATIBLE_UI, CoinFeature.EVM_COMPATIBLE_WP, CoinFeature.SUPPORTS_ERC20, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ] ), account( @@ -2895,6 +2898,7 @@ export const allCoinsAndTokens = [ CoinFeature.EVM_COMPATIBLE_IMS, CoinFeature.EVM_COMPATIBLE_UI, CoinFeature.EVM_COMPATIBLE_WP, + CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX, ] ), diff --git a/modules/statics/src/networks.ts b/modules/statics/src/networks.ts index 6deba319f85..72cc3d8a513 100644 --- a/modules/statics/src/networks.ts +++ b/modules/statics/src/networks.ts @@ -2761,6 +2761,7 @@ class KatanaEth extends Mainnet implements EthereumNetwork { accountExplorerUrl = 'https://katanascan.com/address/'; chainId = 747474; nativeCoinOperationHashPrefix = '747474'; + batcherContractAddress = '0x3e1e5d78e44f15593b3b61ed278f12c27f0ff33e'; } class KatanaEthTestnet extends Testnet implements EthereumNetwork { @@ -2770,6 +2771,7 @@ class KatanaEthTestnet extends Testnet implements EthereumNetwork { accountExplorerUrl = 'https://explorer-bokuto.katanarpc.com/address/'; chainId = 737373; nativeCoinOperationHashPrefix = '737373'; + batcherContractAddress = '0x3e1e5d78e44f15593b3b61ed278f12c27f0ff33e'; } class PrividiumETH extends Mainnet implements EthereumNetwork { diff --git a/modules/statics/test/unit/coins.ts b/modules/statics/test/unit/coins.ts index 304dbfa2018..0e3de08ef3f 100644 --- a/modules/statics/test/unit/coins.ts +++ b/modules/statics/test/unit/coins.ts @@ -1447,6 +1447,19 @@ describe('TSS Verify Use Serialized Tx Hex Feature', () => { }); }); + it('should have TSS_VERIFY_USE_SERIALIZED_TX_HEX on the non-EIP-1559 EVM chains', () => { + // Their signableHex is the EIP-155 preimage RLP([...,chainId,0,0]), which ethereumjs cannot + // parse as a transaction, so TSS verification must decode serializedTxHex instead. + const legacyEvmChains = ['kavaevm', 'tkavaevm', 'prividiumeth', 'tprividiumeth']; + legacyEvmChains.forEach((coinName) => { + const coin = coins.get(coinName); + coin.features.includes(CoinFeature.EIP1559).should.eql(false, `${coinName} must not claim EIP-1559`); + coin.features + .includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX) + .should.eql(true, `${coinName} must verify serializedTxHex`); + }); + }); + it('should have TSS_VERIFY_USE_SERIALIZED_TX_HEX on BSC and XDC tokens', () => { coins.get('bsc:busd').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true); coins.get('xdc:usdc').features.includes(CoinFeature.TSS_VERIFY_USE_SERIALIZED_TX_HEX).should.eql(true); diff --git a/modules/statics/test/unit/networks.ts b/modules/statics/test/unit/networks.ts index 8c713cb5700..59d1862bd10 100644 --- a/modules/statics/test/unit/networks.ts +++ b/modules/statics/test/unit/networks.ts @@ -103,6 +103,15 @@ Object.entries(Networks).forEach(([category, networks]) => { }); }); +describe('Katana Network', function () { + const TRUSTED_BATCHER_ADDRESS = '0x3e1e5d78e44f15593b3b61ed278f12c27f0ff33e'; + + it('should verify batch prebuilds against the deployed batcher on mainnet and testnet', function () { + Networks.main.katanaeth.batcherContractAddress.should.equal(TRUSTED_BATCHER_ADDRESS); + Networks.test.katanaeth.batcherContractAddress.should.equal(TRUSTED_BATCHER_ADDRESS); + }); +}); + describe('Cosmos-family addressPrefix', function () { const cases: Array<[string, AccountNetwork, string]> = [ ['main.atom', Networks.main.atom, 'cosmos'],