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'],