diff --git a/modules/abstract-eth/README.md b/modules/abstract-eth/README.md index 856c0af47ca..1a54297486b 100644 --- a/modules/abstract-eth/README.md +++ b/modules/abstract-eth/README.md @@ -1 +1,23 @@ # BitGo abstract-eth + +## EVM TSS transaction verification + +`verifyTssTransaction` decodes the EVM bytes about to be signed and compares `to` / `value` / +`data` with the user intent before the user key share signs (TOB-BITGOEDMPC-2 / INFOSEC-671). + +- The caller's own `txParams` are authoritative whenever they declare `type` or `recipients`. A + stored tx request intent is consulted only when the caller declared nothing (pending-approval + signing); server data never fills absent caller calldata or replaces the declared token. +- The asset is resolved locally from the recipient's `tokenAddress` / `tokenName`, the top-level + `tokenName` and the coin instance. Unresolvable or contradictory identity fails closed, and the + calldata shape must match the asset: a native send, an ERC-20 / ERC-7984 transfer, or one batch + that moves a single asset with the matching batch method. +- The bytes are decoded once, each check returns findings, and one policy step decides warn or + throw. Fatal findings (native destination and value, ERC-20 recipient and amount, and the + consolidation target when `consolidationToBaseAddress` is set) always throw. Every other + finding is reported through `console.warn` and the optional `onVerificationWarning` callback; + pass `strictTssVerification: true` to `BitGoAPI` to make them hard failures. +- A consolidation must pay the wallet base address, and an ERC-20 consolidation must not carry + native value. +- `contractCall` and `customTx` intents always throw on EVM, and stored `defi-redeem` intents + are rejected because no handler exists for them yet. diff --git a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts index ff45cad8def..c5dea35e2ed 100644 --- a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts +++ b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts @@ -54,8 +54,23 @@ import { DeriveAddressResult, NO_RECIPIENT_TX_TYPES, CoinWithSignableConsistency, + TssVerificationWarning, } from '@bitgo/sdk-core'; import { getDerivationPath } from '@bitgo/sdk-lib-mpc'; +import { + buildEvmTssIntentContext, + callerDeclaresIntent, + canonicalizeEvmTssIntentType, + DecodedEvmTx, + evaluateEvmTssIntent, + isUnsupportedEvmTssIntentType, + mapDefiParamsFromIntent, + resolveCoinTokenContractAddress, + selectDeclaredIntent, + TssFinding, + TssFindingSeverity, + verifyConsolidation, +} from './tssVerification'; import { bip32 } from '@bitgo/secp256k1'; import { BaseCoin as StaticsBaseCoin, @@ -3550,15 +3565,37 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem } /** + * Verify that a TSS transaction prebuild complies with the original intention before the + * user key share signs it (TOB-BITGOEDMPC-2 / INFOSEC-671). + * + * One flow: the intent is normalized once, the bytes are decoded once, and small checks per + * operation return findings. The intent type picks the checks; each accepts a closed set of + * decoded transaction shapes and anything unmatched is a finding. Fatal findings (the checks + * that historically threw) throw at once, including on the pending-approval path. Every other + * finding goes through {@link applyTssVerificationPolicy}: `console.warn` plus the optional + * `onVerificationWarning` callback, or a hard failure when the client opts into `BitGoAPI`'s + * `strictTssVerification`. + * * @param {VerifyEthTransactionOptions} params * @param {TransactionParams} params.txParams - params object passed to send * @param {TransactionPrebuild} params.txPrebuild - prebuild object returned by server * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against + * @param {PopulatedIntent} params.intent - stored tx request intent (pending-approval path) * @returns {boolean} * @throws {TxIntentMismatchRecipientError} if transaction recipients don't match user intent */ async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { - const { txParams, txPrebuild, wallet } = params; + const ethNetwork = this.getNetwork(); + const { txParams, txPrebuild, wallet, intent } = 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. See selectDeclaredIntent. + const { callerParams, storedIntent } = selectDeclaredIntent({ + txParams, + callerTxParams: params.callerTxParams, + intent, + }); // Helper to throw TxIntentMismatchRecipientError with recipient details const throwRecipientMismatch = async (message: string, mismatchedRecipients: Recipient[]): Promise => { @@ -3578,6 +3615,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem !( txParams.prebuildTx?.consolidateId || txPrebuild?.consolidateId || + storedIntent?.consolidateId || txParams.stakingRequestId || txParams.prebuildTx?.stakingRequestId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) @@ -3593,91 +3631,98 @@ 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; - } + // Canonical verifier intent type, derived only from the source that speaks for the user. + // The caller's own `type` wins and is never defaulted from the stored intent: on a direct + // send `txParams.type` is the caller's alone, and a caller that declared recipients without a + // type is making a payment. Only when the caller declared nothing does the stored intent + // type apply: resolveEffectiveTxParams has already put the raw stored spelling (payment, + // defi-approve, …) in txParams.type, which is normalized here. + const explicitType = storedIntent + ? canonicalizeEvmTssIntentType(txParams.type) || canonicalizeEvmTssIntentType(storedIntent.intentType) + : canonicalizeEvmTssIntentType(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 handler. + const consolidateId = callerDeclaresIntent(callerParams) + ? undefined + : txParams.prebuildTx?.consolidateId || txPrebuild?.consolidateId || storedIntent?.consolidateId || undefined; + const isConsolidation = + !!params.verification?.consolidationToBaseAddress || explicitType === 'consolidate' || !!consolidateId; + const canonicalType = explicitType || (isConsolidation ? 'consolidate' : 'transfer'); + + // contractCall / customTx cannot be created on EVM and carry arbitrary calldata with no + // intent shape to verify — never let them reach signing on this path. + if (isUnsupportedEvmTssIntentType(canonicalType)) { + throw new Error(`unsupported intent type for EVM TSS: ${canonicalType}`); + } + + // Decode the bytes once; every check below reads these same fields. + let decodedTx: DecodedEvmTx | undefined; + let decodeError: Error | undefined; + try { + decodedTx = await this.decodeTssTransaction(txPrebuild.txHex); + } catch (e) { + decodeError = e as Error; + } + const requireDecoded = (): DecodedEvmTx => { + if (!decodedTx) { + throw decodeError; + } + return decodedTx; + }; + const raise = (finding: TssFinding): Promise => + throwRecipientMismatch(finding.reason, [ + { address: finding.decoded.to ?? '', amount: finding.decoded.value ?? '' }, + ]); + const raiseFatal = async (findings: TssFinding[]): Promise => { + const fatal = findings.find((finding) => finding.severity === 'fatal'); + if (fatal) { + await raise(fatal); + } + }; - 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() }] - ); - } + const coinBaseAddress = wallet.coinSpecific()?.baseAddress; + const baseAddress = typeof coinBaseAddress === 'string' ? coinBaseAddress : undefined; + const ctx = buildEvmTssIntentContext({ + intentType: canonicalType, + txParams, + callerParams, + storedIntent, + coinTokenContractAddress: resolveCoinTokenContractAddress(this.getChain()), + baseAddress, + batcherAddress: ethNetwork?.batcherContractAddress, + }); - if (expectedRecipientAddress !== addHexPrefix(recipientAddress.toString()).toLowerCase()) { - await throwRecipientMismatch('destination address does not match with the recipient address', [ - { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() }, - ]); - } - } - } + // Operation checks. A single-recipient payment's historic checks need the decoded bytes, so + // bytes that cannot be decoded still fail there as they always did. Fatal findings (those + // historic checks) throw now; the rest wait for the policy step at the end. + if (canonicalType === 'transfer' && ctx.recipients.length === 1) { + requireDecoded(); } + const intentFindings = evaluateEvmTssIntent(ctx, decodedTx ?? { to: '', value: '0', data: '0x' }); + await raiseFatal(intentFindings); - if (txParams.type && ['defiApprove', 'defiDeposit', 'defiWithdraw'].includes(txParams.type)) { + // DeFi vault calldata vs defiParams. Stored defi-* intents reach this branch through the + // intent type mapping, with defiParams reconstructed from the stored intent fields. + if (['defiApprove', 'defiDeposit', 'defiWithdraw'].includes(canonicalType)) { if (!txPrebuild.txHex) { throw new Error('missing txHex in txPrebuild'); } - const baseAddress = wallet.coinSpecific()?.baseAddress; - const defiParams = txParams.defiParams; - if (!baseAddress || !defiParams) { + const defiParams = txParams.defiParams ?? (storedIntent ? mapDefiParamsFromIntent(storedIntent) : undefined); + if (!coinBaseAddress || !defiParams) { await throwRecipientMismatch('DeFi transaction is missing required intent parameters', []); return false; } - const validatedBaseAddress = baseAddress as string; + const validatedBaseAddress = coinBaseAddress as string; const validatedDefiParams = defiParams; - const txBuilder = this.getTransactionBuilder(); - txBuilder.from(txPrebuild.txHex); - const txJson = (await txBuilder.build()).toJson(); + const txJson = requireDecoded(); const mismatch = async (message: string, address = txJson.to, amount = txJson.value): Promise => throwRecipientMismatch(message, [{ address: address || '', amount: amount || '' }]); const data = txJson.data || ''; - if (txParams.type === 'defiApprove') { + if (canonicalType === 'defiApprove') { const selector = addHexPrefix(optionalDeps.ethAbi.methodID('approve', ['address', 'uint256']).toString('hex')); if (!data.startsWith(selector)) { await mismatch('defiApprove transaction must use ERC-20 approve(address,uint256) calldata'); @@ -3690,7 +3735,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem amount.toString() ); } - } else if (txParams.type === 'defiDeposit') { + } else if (canonicalType === 'defiDeposit') { const selector = addHexPrefix(optionalDeps.ethAbi.methodID('deposit', ['uint256', 'address']).toString('hex')); if (!data.startsWith(selector)) { await mismatch('defiDeposit transaction must use deposit(uint256,address) calldata'); @@ -3744,7 +3789,7 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem } } - if (txParams.type === 'wrapApprove') { + if (canonicalType === 'wrapApprove') { await this.verifyWrapApproveTransaction(params, throwRecipientMismatch); } else { // Declared as anything else: only an approve of a registered wrapper is rejected (see the method doc); @@ -3752,55 +3797,85 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin implem await this.assertWrapperApproveIsDeclared(params, throwRecipientMismatch); } - // Verify consolidation transactions send to base address - if (params.verification?.consolidationToBaseAddress) { - const coinSpecific = wallet.coinSpecific(); - if (!coinSpecific || !coinSpecific.baseAddress) { - throw new Error('Unable to determine base address for consolidation'); - } - const baseAddress = coinSpecific.baseAddress; - - if (!txPrebuild.txHex) { - throw new Error('missing txHex in txPrebuild'); + // Consolidations must pay the wallet base address. The check is fatal when the caller sets + // consolidationToBaseAddress, as it always was; for consolidate intents and consolidateId + // flows it is a new check that follows the warn / strict policy. + const consolidationFindings: TssFinding[] = []; + if (isConsolidation) { + const severity: TssFindingSeverity = params.verification?.consolidationToBaseAddress ? 'fatal' : 'new'; + try { + if (!baseAddress) { + throw new Error('Unable to determine base address for consolidation'); + } + const tx = requireDecoded(); + if (!tx.to) { + throw new Error('Consolidation transaction is missing recipient address'); + } + consolidationFindings.push(...verifyConsolidation(tx, baseAddress, severity)); + } catch (e) { + if (severity === 'fatal') { + throw e; + } + consolidationFindings.push({ + severity, + intentType: 'consolidate', + reason: `consolidation destination could not be verified: ${(e as Error).message}`, + expected: { to: baseAddress ?? '' }, + decoded: {}, + }); } + await raiseFatal(consolidationFindings); + } - 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'); - } + const decodeFindings: TssFinding[] = decodedTx + ? [] + : [ + { + severity: 'new', + intentType: canonicalType, + reason: txPrebuild.txHex + ? `transaction bytes could not be decoded to verify intent: ${decodeError?.message}` + : 'missing txHex in txPrebuild: transaction bytes cannot be verified against intent', + expected: {}, + decoded: {}, + }, + ]; - const erc20TransferSelector = addHexPrefix( - optionalDeps.ethAbi.methodID('transfer', ['address', 'uint256']).toString('hex') - ); + await this.applyTssVerificationPolicy([...consolidationFindings, ...decodeFindings, ...intentFindings], raise); + return true; + } - 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 }, - ]); - } + /** Decodes the TSS transaction bytes once, so every verifier check reads the same fields. */ + private async decodeTssTransaction(txHex: string | undefined): 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' }; + } - return true; + /** + * The one place that decides warn or throw for findings that are not fatal: they throw when + * the client sets `strictTssVerification`, and otherwise go to `console.warn` and the + * optional `onVerificationWarning` callback. + */ + private async applyTssVerificationPolicy( + findings: TssFinding[], + raise: (finding: TssFinding) => Promise + ): Promise { + if (!findings.length) { + return; + } + if (this.bitgo?.strictTssVerification) { + await raise(findings[0]); + } + for (const { intentType, reason, expected, decoded } of findings) { + const warning: TssVerificationWarning = { intentType, reason, expected, decoded }; + console.warn(`[tss verification] ${reason}`, JSON.stringify({ expected, decoded })); + this.bitgo?.onVerificationWarning?.(warning); + } } /** diff --git a/modules/abstract-eth/src/lib/iface.ts b/modules/abstract-eth/src/lib/iface.ts index fd5c6cdc000..77bb8640555 100644 --- a/modules/abstract-eth/src/lib/iface.ts +++ b/modules/abstract-eth/src/lib/iface.ts @@ -149,6 +149,21 @@ export interface NativeTransferData extends TransferData { data: string; } +export interface BatchTransferRecipient { + address: string; + amount: string; +} + +/** Decoded inner recipients of batch(address[],uint256[]) batcher calldata. */ +export interface BatchTransferData { + recipients: BatchTransferRecipient[]; +} + +/** Decoded batchTransferFrom(address,address[],uint256[]) batcher calldata. */ +export interface BatchTransferFromData extends BatchTransferData { + tokenContractAddress: string; +} + export interface WalletInitializationData { salt?: string; owners: string[]; diff --git a/modules/abstract-eth/src/lib/utils.ts b/modules/abstract-eth/src/lib/utils.ts index 95dc0ca69d9..a47efbebe84 100644 --- a/modules/abstract-eth/src/lib/utils.ts +++ b/modules/abstract-eth/src/lib/utils.ts @@ -32,6 +32,8 @@ import { } from '@bitgo/sdk-core'; import { + BatchTransferData, + BatchTransferFromData, ERC1155TransferData, ERC721TransferData, FlushTokensData, @@ -91,6 +93,10 @@ import { confidentialTransferWithProofMethodId, confidentialTransferWithProofTypes, confidentialTransferNoProofMethodId, + batchMethodId, + batchMethodTypes, + batchTransferFromMethodId, + batchTransferFromMethodTypes, } from './walletUtil'; import { EthTransactionData } from './types'; import { @@ -479,6 +485,65 @@ export function decodeTransferData(data: string, isFirstSigner?: boolean): Trans } } +/** + * Decode batch(address[],uint256[]) calldata used for native batch sends to the batcher contract. + * + * @param data Hex string starting with the batch method selector + * @returns Decoded recipients and amounts in the order they appear in the calldata + */ +export function decodeBatchTransferData(data: string): BatchTransferData { + if (!data.toLowerCase().startsWith(batchMethodId)) { + throw new BuildTransactionError(`Invalid batch transfer bytecode: ${data}`); + } + const [addresses, amounts] = getRawDecoded(batchMethodTypes, getBufferedByteCode(batchMethodId, data)); + if (!Array.isArray(addresses) || !Array.isArray(amounts)) { + throw new BuildTransactionError(`Invalid batch transfer bytecode: ${data}`); + } + if (addresses.length !== amounts.length) { + throw new BuildTransactionError( + `Mismatched batch address/amount array lengths: ${addresses.length} vs ${amounts.length}` + ); + } + return { + recipients: addresses.map((addr, i) => ({ + address: addHexPrefix(addr as string), + amount: new BigNumber(bufferToHex(amounts[i] as Buffer)).toFixed(), + })), + }; +} + +/** + * Decode batchTransferFrom(address,address[],uint256[]) calldata used for token batch sends + * to the batcher contract. + * + * @param data Hex string starting with the batchTransferFrom method selector + * @returns Decoded token contract, recipients and amounts in calldata order + */ +export function decodeBatchTransferFromData(data: string): BatchTransferFromData { + if (!data.toLowerCase().startsWith(batchTransferFromMethodId)) { + throw new BuildTransactionError(`Invalid batchTransferFrom bytecode: ${data}`); + } + const [tokenContractAddress, addresses, amounts] = getRawDecoded( + batchTransferFromMethodTypes, + getBufferedByteCode(batchTransferFromMethodId, data) + ); + if (!Array.isArray(addresses) || !Array.isArray(amounts)) { + throw new BuildTransactionError(`Invalid batchTransferFrom bytecode: ${data}`); + } + if (addresses.length !== amounts.length) { + throw new BuildTransactionError( + `Mismatched batchTransferFrom address/amount array lengths: ${addresses.length} vs ${amounts.length}` + ); + } + return { + tokenContractAddress: addHexPrefix(tokenContractAddress as string), + recipients: addresses.map((addr, i) => ({ + address: addHexPrefix(addr as string), + amount: new BigNumber(bufferToHex(amounts[i] as Buffer)).toFixed(), + })), + }; +} + /** * Decode the given ABI-encoded transfer data for the sendMultisigToken function and return parsed fields * diff --git a/modules/abstract-eth/src/lib/walletUtil.ts b/modules/abstract-eth/src/lib/walletUtil.ts index 8d5891d0162..23349874561 100644 --- a/modules/abstract-eth/src/lib/walletUtil.ts +++ b/modules/abstract-eth/src/lib/walletUtil.ts @@ -1,5 +1,13 @@ export const sendMultisigMethodId = '0x39125215'; export const sendMultisigTokenMethodId = '0x0dcd7a6c'; +// Selector for batch(address[],uint256[]) used by batcher contract sends (native batches). +export const batchMethodId = '0xc00c4e9e'; +export const batchMethodName = 'batch'; +export const batchMethodTypes = ['address[]', 'uint256[]']; +// Selector for batchTransferFrom(address,address[],uint256[]) used for token batches. +export const batchTransferFromMethodId = '0x4885b254'; +export const batchTransferFromMethodName = 'batchTransferFrom'; +export const batchTransferFromMethodTypes = ['address', 'address[]', 'uint256[]']; export const v1CreateForwarderMethodId = '0xfb90b320'; export const v4CreateForwarderMethodId = '0x13b2f75c'; export const v1WalletInitializationFirstBytes = '0x60806040'; diff --git a/modules/abstract-eth/src/tssVerification.ts b/modules/abstract-eth/src/tssVerification.ts new file mode 100644 index 00000000000..72dd6652928 --- /dev/null +++ b/modules/abstract-eth/src/tssVerification.ts @@ -0,0 +1,1109 @@ +/** + * Fail-closed EVM TSS intent verification. + * + * `verifyTssTransaction` must not let a user key share sign EVM bytes unless the decoded + * `to` / `value` / `data` match the user intent (TOB-BITGOEDMPC-2 / INFOSEC-671). The intent + * type picks the handler; each handler accepts a closed set of decoded transaction shapes + * and anything unmatched is a violation. Types are never derived from calldata: selectors + * are shared across intents and `payment + data` carries arbitrary calldata. + * + * The checks only return findings; `AbstractEthLikeNewCoins.verifyTssTransaction` applies the + * policy in one place. A `fatal` finding (a check that already threw before this module existed) + * always throws. `new` and `unhandled` findings are reported through `console.warn` plus an + * optional `onVerificationWarning` callback, and throw when the client opts into + * `strictTssVerification`. + */ +import { + DefiIntentParams, + ITransactionRecipient, + PopulatedIntent, + TssVerificationMismatch, + TssVerificationWarning, +} from '@bitgo/sdk-core'; +import { coins, ContractAddressDefinedToken } from '@bitgo/statics'; +import { Buffer } from 'buffer'; +import { addHexPrefix, bufferToHex } from 'ethereumjs-util'; +import { + batchMethodId, + batchTransferFromMethodId, + BatchTransferData, + BatchTransferFromData, + confidentialTransferNoProofMethodId, + confidentialTransferNoProofTypes, + confidentialTransferWithProofMethodId, + confidentialTransferWithProofTypes, + decodeApproveCalldata, + decodeBatchTransferData, + decodeBatchTransferFromData, + getBufferedByteCode, + getRawDecoded, +} from './lib'; + +/** ERC-20 `transfer(address,uint256)` selector. */ +export const erc20TransferMethodId = '0xa9059cbb'; +/** Maximum ERC-20 allowance: uint256 max. */ +export const MAX_UINT256 = ((1n << 256n) - 1n).toString(); + +/** + * Canonical verifier intent types with a fail-closed handler. Anything outside this set + * (staking, `*WithCallData`, `acceleration`, wrap / unwrap-native, `enableToken`, + * `bridgeFunds`, `defiRedeem`, …) has no handler and is flagged instead of silently passing. + */ +export type EvmTssIntentType = + | 'transfer' + | 'transferToken' + | 'tokenApproval' + | 'fillNonce' + | 'consolidate' + | 'defiApprove' + | 'defiDeposit' + | 'defiWithdraw' + | 'wrapApprove' + | 'contractCall' + | 'customTx'; + +/** + * Stored intent types that cannot be created on EVM and must never reach signing there. + * `contractCall` / `customTx` carry arbitrary calldata with no intent shape to verify. + */ +const UNSUPPORTED_EVM_INTENT_TYPES = new Set(['contractCall', 'customTx']); + +/** Stored (kebab-case) intent spellings mapped to canonical verifier types. */ +const INTENT_TYPE_ALIASES: Record = { + payment: 'transfer', + 'transfer-token': 'transferToken', + // `Wallet.prebuildTransactionTxRequests` switches on the lowercase spelling for token sends. + transfertoken: 'transferToken', + 'token-approval': 'tokenApproval', + 'fill-nonce': 'fillNonce', + 'bridge-funds': 'bridgeFunds', + 'enable-token': 'enableToken', + 'custom-tx': 'customTx', + 'contract-call': 'contractCall', + 'defi-approve': 'defiApprove', + 'defi-deposit': 'defiDeposit', + 'defi-withdraw': 'defiWithdraw', + 'defi-redeem': 'defiRedeem', + 'wrap-approve': 'wrapApprove', + 'wrap-native': 'wrapNative', + 'unwrap-native': 'unwrapNative', + 'finalize-unwrap': 'finalizeUnwrap', + 'stake-with-call-data': 'stakeWithCallData', + 'unstake-with-call-data': 'unstakeWithCallData', +}; + +/** Transaction fields decoded from the bytes about to be signed. */ +export interface DecodedEvmTx { + to: string; + value: string; + data: string; +} + +/** + * How a finding is enforced. `fatal` always throws: these are the checks that threw before + * fail-closed verification existed. `new` (a mismatch on a handled intent type) and `unhandled` + * (an intent type with no handler) follow the client's warn / strict policy. + */ +export type TssFindingSeverity = 'fatal' | 'new' | 'unhandled'; + +/** One rule the transaction bytes broke, with the severity that decides warn or throw. */ +export interface TssFinding extends TssVerificationWarning { + severity: TssFindingSeverity; +} + +/** One recipient of the user intent, flattened from txParams or the stored tx request intent. */ +export interface EvmTssExpectedRecipient { + address: string; + amount: string; + /** Raw calldata the intent declares for this recipient (WalletConnect / dApp flows). */ + data?: string; + tokenName?: string; + tokenContractAddress?: string; +} + +export interface EvmTssIntentContext { + /** Canonical verifier intent type (after {@link canonicalizeEvmTssIntentType}). */ + intentType: string; + recipients: EvmTssExpectedRecipient[]; + /** Token name declared by the caller (or, on the pending-approval path, the stored intent). */ + tokenName?: string; + /** + * Contract of the token the coin instance itself represents (e.g. `bitgo.coin('eth:usdc')`), + * undefined for base coins. Locally selected: it is the asset of any payment that declares no + * token of its own. + */ + coinTokenContractAddress?: string; + /** Wallet base address (`wallet.coinSpecific().baseAddress`). */ + baseAddress?: string; + /** Network batcher contract from statics (`network.batcherContractAddress`). */ + batcherAddress?: string; +} + +function kebabToCamel(value: string): string { + return value.replace(/-([a-z0-9])/g, (_, c: string) => c.toUpperCase()); +} + +/** + * Map a declared transaction type onto the canonical verifier type. Caller-supplied + * `txParams.type` and stored `intent.intentType` both flow through here, so kebab-case + * stored spellings (`payment`, `defi-approve`, …) land on their canonical handlers. + */ +export function canonicalizeEvmTssIntentType(rawType?: string): string { + if (!rawType) { + return ''; + } + const trimmed = rawType.trim(); + return INTENT_TYPE_ALIASES[trimmed.toLowerCase()] ?? INTENT_TYPE_ALIASES[trimmed] ?? kebabToCamel(trimmed); +} + +/** True when the canonical type must never be signed on EVM chains. */ +export function isUnsupportedEvmTssIntentType(canonicalType: string): boolean { + return UNSUPPORTED_EVM_INTENT_TYPES.has(canonicalType as EvmTssIntentType); +} + +function normalizeAddress(address?: string): string { + return (address ?? '').toLowerCase(); +} + +function toAddressString(value: unknown): string { + const raw = Buffer.isBuffer(value) ? bufferToHex(value as Buffer) : String(value); + return addHexPrefix(raw).toLowerCase(); +} + +function toBigInt(value: unknown): bigint { + if (typeof value === 'bigint') { + return value; + } + if (Buffer.isBuffer(value)) { + return BigInt(bufferToHex(value as Buffer)); + } + return BigInt(String(value)); +} + +function isEmptyCalldata(data?: string): boolean { + return !data || data.toLowerCase() === '0x'; +} + +function normalizeCalldata(data?: string): string { + return (data ?? '0x').toLowerCase(); +} + +function valuesEqual(a: unknown, b: unknown): boolean { + try { + return toBigInt(a) === toBigInt(b); + } catch { + return String(a) === String(b); + } +} + +function violation( + ctx: Pick, + reason: string, + expected: TssVerificationMismatch, + decoded: TssVerificationMismatch, + severity: TssFindingSeverity = 'new' +): TssFinding { + return { intentType: ctx.intentType, reason, expected, decoded, severity }; +} + +/** + * Resolve the on-chain contract of `tokenName` from statics. Returns undefined for unknown + * names and for coins without a contract address (native coins, known statics gaps). + */ +export function resolveTokenContractAddress(tokenName?: string): string | undefined { + if (!tokenName) { + return undefined; + } + try { + const coin = coins.get(tokenName); + return coin instanceof ContractAddressDefinedToken ? coin.contractAddress.toString() : undefined; + } catch { + return undefined; + } +} + +/** + * Contract of the token that `chain` itself represents (`bitgo.coin('eth:usdc')` is the USDC + * contract). Undefined for base coins and for native coins that merely expose an ERC-20 + * interface (`isToken: false`). The coin instance is chosen locally, so it is a trusted source + * of asset identity for payments that name no token of their own. + */ +export function resolveCoinTokenContractAddress(chain?: string): string | undefined { + if (!chain) { + return undefined; + } + try { + const coin = coins.get(chain); + return coin instanceof ContractAddressDefinedToken && coin.isToken ? coin.contractAddress.toString() : undefined; + } catch { + return undefined; + } +} + +/** The asset a payment moves, resolved only from locally declared identity. */ +type PaymentAsset = { kind: 'native' } | { kind: 'token'; contract: string } | { kind: 'unresolved'; reason: string }; + +const EVM_ADDRESS = /^0x[0-9a-fA-F]{40}$/; + +/** + * Resolve the asset a payment is declared to move. Identity comes from the recipient's own + * `tokenAddress` / `tokenName`, the top-level `tokenName`, and the coin instance, and nothing + * else: the context never carries server-supplied token metadata when the caller declared its + * own intent. No declared identity at all means native. Identity that cannot be resolved to a + * contract, or that names two different contracts, is `unresolved`: the intended asset is + * unknown, so no calldata shape can be accepted for it. + */ +function resolveAsset( + ctx: EvmTssIntentContext, + declared: { tokenName?: string; tokenContractAddress?: string } +): PaymentAsset { + const sources: { label: string; contract: string | undefined; native?: boolean }[] = []; + if (declared.tokenContractAddress !== undefined) { + sources.push({ + label: `tokenAddress '${declared.tokenContractAddress}'`, + contract: EVM_ADDRESS.test(declared.tokenContractAddress) ? declared.tokenContractAddress : undefined, + }); + } + for (const name of [declared.tokenName, ctx.tokenName]) { + if (!name) { + continue; + } + let staticsCoin; + try { + staticsCoin = coins.get(name); + } catch { + staticsCoin = undefined; + } + if (staticsCoin && !(staticsCoin instanceof ContractAddressDefinedToken && staticsCoin.isToken)) { + // The name of a native coin (`hteth`, or a native coin that merely exposes an ERC-20 + // interface such as `celo`): the same identity as declaring no token at all. + sources.push({ label: `coin '${name}'`, contract: undefined, native: true }); + } else { + sources.push({ label: `token '${name}'`, contract: resolveTokenContractAddress(name) }); + } + } + if (ctx.coinTokenContractAddress) { + sources.push({ label: 'the coin token', contract: ctx.coinTokenContractAddress }); + } + const tokenSources = sources.filter((source) => !source.native); + if (tokenSources.length === 0) { + return { kind: 'native' }; + } + const missing = tokenSources.find((source) => source.contract === undefined); + if (missing) { + return { + kind: 'unresolved', + reason: `${missing.label} cannot be resolved to a token contract: the intended asset cannot be verified`, + }; + } + const nativeSource = sources.find((source) => source.native); + if (nativeSource) { + return { + kind: 'unresolved', + reason: `${nativeSource.label} and ${tokenSources[0].label} name different assets: the intended asset is ambiguous`, + }; + } + const [first, ...rest] = tokenSources as { label: string; contract: string }[]; + const conflict = rest.find((source) => normalizeAddress(source.contract) !== normalizeAddress(first.contract)); + if (conflict) { + return { + kind: 'unresolved', + reason: `${first.label} and ${conflict.label} name different token contracts: the intended asset is ambiguous`, + }; + } + return { kind: 'token', contract: first.contract }; +} + +type CallerDeclarableParams = { type?: string; tokenName?: string; recipients?: ITransactionRecipient[] }; + +/** True when the caller named a transaction type or recipients, i.e. declared its own intent. */ +export function callerDeclaresIntent(callerParams?: CallerDeclarableParams): boolean { + return !!(callerParams?.type || callerParams?.recipients?.length); +} + +/** + * Decide which source speaks for the user intent. Exactly one does: + * + * - The caller declared a type or recipients: the caller's own params. The stored intent is + * dropped entirely: it must not select the handler, add calldata, name the asset, or supply + * consolidation / DeFi fields. + * - The caller declared nothing (signing a stored tx request, e.g. pending approval): the stored + * intent, which `resolveEffectiveTxParams` has already folded into `txParams`. + * + * `txParams` alone cannot answer this: with a stored `intent` present, `type` and `recipients` + * have already been back-filled into it from server data. `callerTxParams` is the untouched + * caller input (`undefined` when the caller supplied none). Without a stored intent `txParams` + * is the caller input. + */ +export function selectDeclaredIntent(params: { + txParams: T; + callerTxParams?: T; + intent?: PopulatedIntent; +}): { callerParams?: T; storedIntent?: PopulatedIntent } { + // `callerTxParams` is authoritative whenever the signing path supplies it. Without it, a + // stored `intent` means the caller supplied nothing (pending approval), and without an intent + // `txParams` is the caller input (direct `verifyTransaction` callers). + const callerParams = params.callerTxParams ?? (params.intent ? undefined : params.txParams); + const storedIntent = params.intent && !callerDeclaresIntent(callerParams) ? params.intent : undefined; + return { callerParams, storedIntent }; +} + +/** + * Build the handler context from the source chosen by {@link selectDeclaredIntent}: the stored + * intent (via the effective `txParams`) when `storedIntent` is set, otherwise the caller's own + * params. Absent local `data` stays absent and local token identity is never replaced. + */ +export function buildEvmTssIntentContext(params: { + intentType: string; + /** Effective params: back-filled from the stored intent when `storedIntent` is set. */ + txParams: CallerDeclarableParams; + /** The caller's own declared params. */ + callerParams?: CallerDeclarableParams; + /** Set only when the caller declared nothing and the stored intent speaks for the user. */ + storedIntent?: PopulatedIntent; + coinTokenContractAddress?: string; + baseAddress?: string; + batcherAddress?: string; +}): EvmTssIntentContext { + const { storedIntent } = params; + const source = storedIntent ? params.txParams : params.callerParams; + + const recipients: EvmTssExpectedRecipient[] = (source?.recipients ?? []).map((recipient, i) => { + const tokenContractAddress = + recipient.tokenAddress ?? storedIntent?.recipients?.[i]?.tokenData?.tokenContractAddress; + return { + address: recipient.address, + amount: String(recipient.amount), + ...(recipient.data !== undefined && { data: recipient.data }), + ...(recipient.tokenName !== undefined && { tokenName: recipient.tokenName }), + ...(tokenContractAddress !== undefined && { tokenContractAddress }), + }; + }); + + return { + intentType: params.intentType, + recipients, + tokenName: source?.tokenName ?? storedIntent?.tokenName, + coinTokenContractAddress: params.coinTokenContractAddress, + baseAddress: params.baseAddress, + batcherAddress: params.batcherAddress, + }; +} + +/** + * Map stored DeFi intent fields onto the `defiParams` consumed by the DeFi calldata checks. + * `shareTokenAmount` is the withdraw (share) amount; `amount` covers approve / deposit. + */ +export function mapDefiParamsFromIntent(intent: PopulatedIntent): DefiIntentParams | undefined { + const canonicalType = canonicalizeEvmTssIntentType(intent.intentType); + const rawAmount = + canonicalType === 'defiWithdraw' + ? intent.shareTokenAmount ?? intent.amount + : intent.amount ?? intent.shareTokenAmount; + const amount = typeof rawAmount === 'object' ? rawAmount?.value : rawAmount; + if (amount === undefined || amount === null) { + return undefined; + } + return { + vaultId: intent.vaultId ?? '', + amount: String(amount), + ...(intent.operationId !== undefined && { operationId: intent.operationId }), + }; +} + +/** + * R1 — one recipient that declares raw calldata (WalletConnect / dApp): `to`, `value` and + * `data` must each be exactly equal to the intent. + */ +function verifyExactCall(ctx: EvmTssIntentContext, tx: DecodedEvmTx, recipient: EvmTssExpectedRecipient): TssFinding[] { + const warnings: TssFinding[] = []; + const expected = { to: recipient.address, value: recipient.amount, data: recipient.data }; + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + if (normalizeAddress(tx.to) !== normalizeAddress(recipient.address)) { + warnings.push( + violation(ctx, 'transaction to does not match the recipient address given by the client', expected, decoded) + ); + } + if (!valuesEqual(tx.value, recipient.amount)) { + warnings.push(violation(ctx, 'transaction value does not match the amount given by the client', expected, decoded)); + } + if (normalizeCalldata(tx.data) !== normalizeCalldata(recipient.data)) { + warnings.push( + violation(ctx, 'transaction calldata does not match the calldata given by the client', expected, decoded) + ); + } + return warnings; +} + +/** + * The payment checks that threw before fail-closed verification existed, kept fatal and in their + * original order (amount, then destination). Empty calldata must match `to` and `value`; ERC-20 + * `transfer(address,uint256)` calldata must match the inner recipient and amount. A WalletConnect + * recipient whose own calldata is an ERC-20 transfer supplies that expectation instead. + */ +function verifyHistoricPayment( + ctx: EvmTssIntentContext, + tx: DecodedEvmTx, + recipient: EvmTssExpectedRecipient +): TssFinding[] { + const findings: TssFinding[] = []; + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + if (isEmptyCalldata(tx.data)) { + const expected = { to: recipient.address, value: recipient.amount, data: '0x' }; + if (!valuesEqual(tx.value, recipient.amount)) { + findings.push( + violation( + ctx, + 'the transaction amount in txPrebuild does not match the value given by client', + expected, + decoded, + 'fatal' + ) + ); + } + if (normalizeAddress(tx.to) !== normalizeAddress(recipient.address)) { + findings.push( + violation(ctx, 'destination address does not match with the recipient address', expected, decoded, 'fatal') + ); + } + return findings; + } + if (!normalizeCalldata(tx.data).startsWith(erc20TransferMethodId)) { + return findings; + } + + const [innerRecipient, innerAmount] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode(erc20TransferMethodId, tx.data) + ); + const decodedInner = { ...decoded, to: toAddressString(innerRecipient), value: toBigInt(innerAmount).toString() }; + let expected = { to: recipient.address, value: recipient.amount, data: recipient.data }; + if (recipient.data && normalizeCalldata(recipient.data).startsWith(erc20TransferMethodId)) { + const [declaredRecipient, declaredAmount] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode(erc20TransferMethodId, recipient.data) + ); + expected = { ...expected, to: toAddressString(declaredRecipient), value: toBigInt(declaredAmount).toString() }; + } + if (!valuesEqual(decodedInner.value, expected.value)) { + findings.push( + violation( + ctx, + 'the transaction amount in txPrebuild does not match the value given by client', + expected, + decodedInner, + 'fatal' + ) + ); + } + if (decodedInner.to !== normalizeAddress(expected.to)) { + findings.push( + violation(ctx, 'destination address does not match with the recipient address', expected, decodedInner, 'fatal') + ); + } + return findings; +} + +/** + * R3 — one ERC-20 recipient: `to` must be the token contract, `value` must be zero and the + * decoded `transfer(address,uint256)` recipient and amount must match the intent. + * ERC-7984 `confidentialTransfer` (`0x5bebed7e` / `0x2fb74e62`) is checked on the recipient + * only, since the amount is encrypted. On a payment the ERC-20 recipient and amount are already + * fatal checks (`verifyHistoricPayment`), so `checkInnerTransfer` is false there. + */ +function verifyTokenTransfer( + ctx: EvmTssIntentContext, + tx: DecodedEvmTx, + recipient: EvmTssExpectedRecipient, + tokenContract: string, + checkInnerTransfer: boolean +): TssFinding[] { + const warnings: TssFinding[] = []; + const data = normalizeCalldata(tx.data); + const selector = data.slice(0, 10); + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + const expected = { to: tokenContract, value: '0', data: recipient.data }; + + if (!valuesEqual(tx.value, '0')) { + warnings.push(violation(ctx, 'token transfer must not carry native value', expected, decoded)); + } + + if (normalizeAddress(tx.to) !== normalizeAddress(tokenContract)) { + warnings.push(violation(ctx, 'transaction to is not the intent token contract', expected, decoded)); + } + + if (selector === erc20TransferMethodId) { + if (!checkInnerTransfer) { + return warnings; + } + let decodedRecipient: string; + let decodedAmount: bigint; + try { + const [innerRecipient, innerAmount] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode(erc20TransferMethodId, tx.data) + ); + decodedRecipient = toAddressString(innerRecipient); + decodedAmount = toBigInt(innerAmount); + } catch (e) { + warnings.push( + violation(ctx, `ERC-20 transfer calldata could not be decoded: ${(e as Error).message}`, expected, decoded) + ); + return warnings; + } + if (decodedRecipient !== normalizeAddress(recipient.address)) { + warnings.push( + violation( + ctx, + 'destination address does not match with the recipient address', + { ...expected, to: recipient.address, value: recipient.amount }, + { ...decoded, to: addHexPrefix(decodedRecipient), value: decodedAmount.toString() } + ) + ); + } + if (!valuesEqual(decodedAmount, recipient.amount)) { + warnings.push( + violation( + ctx, + 'the transaction amount in txPrebuild does not match the value given by client', + { ...expected, to: recipient.address, value: recipient.amount }, + { ...decoded, to: addHexPrefix(decodedRecipient), value: decodedAmount.toString() } + ) + ); + } + return warnings; + } + + if (selector === confidentialTransferNoProofMethodId || selector === confidentialTransferWithProofMethodId) { + const types = + selector === confidentialTransferNoProofMethodId + ? confidentialTransferNoProofTypes + : confidentialTransferWithProofTypes; + let decodedRecipient: string; + try { + const [innerRecipient] = getRawDecoded(types, getBufferedByteCode(selector, tx.data)); + decodedRecipient = toAddressString(innerRecipient); + } catch (e) { + warnings.push( + violation(ctx, `confidentialTransfer calldata could not be decoded: ${(e as Error).message}`, expected, decoded) + ); + return warnings; + } + if (decodedRecipient !== normalizeAddress(recipient.address)) { + warnings.push( + violation( + ctx, + 'confidentialTransfer recipient does not match the recipient address given by the client', + { ...expected, to: recipient.address }, + { ...decoded, to: addHexPrefix(decodedRecipient) } + ) + ); + } + return warnings; + } + + warnings.push( + violation( + ctx, + 'token transfer must use ERC-20 transfer(address,uint256) or ERC-7984 confidentialTransfer calldata', + expected, + decoded + ) + ); + return warnings; +} + +/** + * R4 — more than one recipient: `to` must be the network batcher contract and the decoded + * batch arrays must match the intent 1:1. Native `batch(address[],uint256[])` carries + * `value` = the sum of amounts; token `batchTransferFrom(address,address[],uint256[])` + * names the intent token contract and carries `value` = 0. + */ +function verifyBatch(ctx: EvmTssIntentContext, tx: DecodedEvmTx): TssFinding[] { + const warnings: TssFinding[] = []; + const data = normalizeCalldata(tx.data); + const selector = data.slice(0, 10); + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + const expectedTotal = ctx.recipients.reduce((sum, r) => sum + toBigInt(r.amount), 0n).toString(); + const expected = { to: ctx.batcherAddress, value: expectedTotal, data: undefined }; + + if (!ctx.batcherAddress) { + warnings.push( + violation( + ctx, + 'no batcher contract address is configured for this network; batch payment cannot be verified', + expected, + decoded + ) + ); + return warnings; + } + if (normalizeAddress(tx.to) !== normalizeAddress(ctx.batcherAddress)) { + warnings.push( + violation(ctx, 'batch transaction to does not match the network batcher contract', expected, decoded) + ); + } + const assets = ctx.recipients.map((recipient) => resolveAsset(ctx, recipient)); + const unresolvedAsset = assets.find((asset) => asset.kind === 'unresolved'); + if (unresolvedAsset?.kind === 'unresolved') { + warnings.push(violation(ctx, `batch payment: ${unresolvedAsset.reason}`, expected, decoded)); + return warnings; + } + const batchAsset = assets[0]; + const mixedAssets = assets.some( + (asset) => + asset.kind !== batchAsset.kind || + (asset.kind === 'token' && + batchAsset.kind === 'token' && + normalizeAddress(asset.contract) !== normalizeAddress(batchAsset.contract)) + ); + if (mixedAssets) { + warnings.push( + violation( + ctx, + 'batch payment recipients declare more than one asset: cannot verify one batch call', + expected, + decoded + ) + ); + return warnings; + } + + const compareRecipients = ( + recipients: { address: string; amount: string }[], + valueExpected: string, + valueReason: string + ): void => { + if (recipients.length !== ctx.recipients.length) { + warnings.push( + violation( + ctx, + `batch calldata recipient count (${recipients.length}) does not match intent recipients (${ctx.recipients.length})`, + expected, + decoded + ) + ); + return; + } + for (let i = 0; i < recipients.length; i++) { + const want = ctx.recipients[i]; + const got = recipients[i]; + if (normalizeAddress(got.address) !== normalizeAddress(want.address) || !valuesEqual(got.amount, want.amount)) { + warnings.push( + violation( + ctx, + `batch calldata entry ${i} does not match intent recipient`, + { ...expected, to: want.address, value: want.amount }, + { ...decoded, to: got.address, value: got.amount } + ) + ); + } + } + if (!valuesEqual(tx.value, valueExpected)) { + warnings.push(violation(ctx, valueReason, { ...expected, value: valueExpected }, decoded)); + } + }; + + if (selector === batchMethodId) { + if (batchAsset.kind !== 'native') { + warnings.push( + violation( + ctx, + 'token batch payment must use batchTransferFrom(address,address[],uint256[]), not native batch(address[],uint256[])', + expected, + decoded + ) + ); + return warnings; + } + let batch: BatchTransferData; + try { + batch = decodeBatchTransferData(tx.data); + } catch (e) { + warnings.push(violation(ctx, `batch calldata could not be decoded: ${(e as Error).message}`, expected, decoded)); + return warnings; + } + compareRecipients( + batch.recipients, + expectedTotal, + 'batch transaction value does not match the sum of intent amounts' + ); + return warnings; + } + + if (selector === batchTransferFromMethodId) { + if (batchAsset.kind !== 'token') { + warnings.push( + violation( + ctx, + 'native batch payment must use batch(address[],uint256[]), not batchTransferFrom(address,address[],uint256[])', + expected, + decoded + ) + ); + return warnings; + } + let batch: BatchTransferFromData; + try { + batch = decodeBatchTransferFromData(tx.data); + } catch (e) { + warnings.push( + violation(ctx, `batchTransferFrom calldata could not be decoded: ${(e as Error).message}`, expected, decoded) + ); + return warnings; + } + if (normalizeAddress(batch.tokenContractAddress) !== normalizeAddress(batchAsset.contract)) { + warnings.push( + violation( + ctx, + 'batchTransferFrom token contract does not match the intent token contract', + { ...expected, to: batchAsset.contract }, + { ...decoded, to: batch.tokenContractAddress } + ) + ); + } + compareRecipients(batch.recipients, '0', 'token batch transaction must not carry native value'); + return warnings; + } + + warnings.push( + violation( + ctx, + 'batch payment must use batch(address[],uint256[]) or batchTransferFrom(address,address[],uint256[]) calldata', + expected, + decoded + ) + ); + return warnings; +} + +/** Payment handler (`transfer` / `payment`) — rules R1–R5. */ +function verifyPayment(ctx: EvmTssIntentContext, tx: DecodedEvmTx): TssFinding[] { + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + if (ctx.recipients.length === 0) { + return [ + violation( + ctx, + 'payment intent has no recipients: the transaction bytes cannot be verified against intent', + {}, + decoded + ), + ]; + } + if (ctx.recipients.length > 1) { + return verifyBatch(ctx, tx); + } + const recipient = ctx.recipients[0]; + return [...verifyHistoricPayment(ctx, tx, recipient), ...verifyPaymentShape(ctx, tx, recipient)]; +} + +/** + * The new checks for a single-recipient payment: the calldata shape must match the declared + * asset. The fields `verifyHistoricPayment` already compares fatally are not compared again. + */ +function verifyPaymentShape( + ctx: EvmTssIntentContext, + tx: DecodedEvmTx, + recipient: EvmTssExpectedRecipient +): TssFinding[] { + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + if (recipient.data !== undefined) { + return verifyExactCall(ctx, tx, recipient); + } + const asset = resolveAsset(ctx, recipient); + if (asset.kind === 'unresolved') { + return [violation(ctx, `payment ${asset.reason}`, { to: recipient.address, value: recipient.amount }, decoded)]; + } + const selector = normalizeCalldata(tx.data).slice(0, 10); + const isTokenShape = + selector === erc20TransferMethodId || + selector === confidentialTransferNoProofMethodId || + selector === confidentialTransferWithProofMethodId; + const isBatchShape = selector === batchMethodId || selector === batchTransferFromMethodId; + if (asset.kind === 'native') { + if (isEmptyCalldata(tx.data)) { + // `to` and `value` of a native send are fatal checks in verifyHistoricPayment. + return []; + } + if (isTokenShape || isBatchShape) { + return [ + violation( + ctx, + 'payment intent declares a native asset but the transaction carries token or batch calldata', + { to: recipient.address, value: recipient.amount, data: '0x' }, + decoded + ), + ]; + } + } else { + if (isEmptyCalldata(tx.data)) { + return [ + violation( + ctx, + 'token payment intent but the transaction is a native send: expected ERC-20 transfer calldata', + { to: asset.contract, value: '0', data: undefined }, + decoded + ), + ]; + } + if (isTokenShape) { + return verifyTokenTransfer(ctx, tx, recipient, asset.contract, false); + } + if (isBatchShape) { + return [ + violation( + ctx, + 'batch calldata cannot be verified against a single declared recipient', + { to: recipient.address, value: recipient.amount }, + decoded + ), + ]; + } + } + // R5 — anything else (e.g. `approve(attacker, MAX)` disguised as a payment) is a violation. + return [ + violation( + ctx, + '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', + { to: recipient.address, value: recipient.amount, data: '0x' }, + decoded + ), + ]; +} + +/** `transferToken` handler — same shape set as R3, exactly one recipient, which must move a token. */ +function verifyTransferToken(ctx: EvmTssIntentContext, tx: DecodedEvmTx): TssFinding[] { + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + if (ctx.recipients.length !== 1) { + return [ + violation( + ctx, + `transferToken intent must declare exactly one token recipient (found ${ctx.recipients.length})`, + {}, + decoded + ), + ]; + } + const recipient = ctx.recipients[0]; + const asset = resolveAsset(ctx, recipient); + if (asset.kind === 'unresolved') { + return [ + violation(ctx, `transferToken ${asset.reason}`, { to: recipient.address, value: recipient.amount }, decoded), + ]; + } + if (asset.kind === 'native') { + return [ + violation( + ctx, + 'transferToken intent names no token: the intended asset cannot be verified', + { to: recipient.address, value: recipient.amount }, + decoded + ), + ]; + } + if (isEmptyCalldata(tx.data)) { + return [ + violation( + ctx, + 'transferToken transaction must use ERC-20 transfer(address,uint256) calldata; expected token calldata, got an empty data payload', + { to: asset.contract, value: '0' }, + decoded + ), + ]; + } + return verifyTokenTransfer(ctx, tx, recipient, asset.contract, true); +} + +/** + * `tokenApproval` handler — `to` must be the token contract and the calldata must be + * `approve(spender = batcher, MAX)`, with `value` = 0. Guards against a substituted + * `approve(attacker, MAX_UINT256)`. + */ +function verifyTokenApproval(ctx: EvmTssIntentContext, tx: DecodedEvmTx): TssFinding[] { + const warnings: TssFinding[] = []; + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + const asset = resolveAsset(ctx, {}); + const tokenContract = asset.kind === 'token' ? asset.contract : undefined; + const expected = { to: tokenContract, value: '0', data: undefined }; + + if (!valuesEqual(tx.value, '0')) { + warnings.push(violation(ctx, 'tokenApproval transaction must not carry native value', expected, decoded)); + } + if (!tokenContract) { + warnings.push( + violation( + ctx, + asset.kind === 'unresolved' + ? `tokenApproval ${asset.reason}` + : `tokenApproval intent names no token contract that can be resolved from statics ('${ + ctx.tokenName ?? '' + }'); transaction to cannot be verified`, + expected, + decoded + ) + ); + } else if (normalizeAddress(tx.to) !== normalizeAddress(tokenContract)) { + warnings.push(violation(ctx, 'tokenApproval transaction to is not the intent token contract', expected, decoded)); + } + + let spender: string; + let amount: string; + try { + const decodedApprove = decodeApproveCalldata(tx.data); + spender = normalizeAddress(decodedApprove.spender); + amount = decodedApprove.amount; + } catch (e) { + warnings.push( + violation( + ctx, + `tokenApproval transaction must use ERC-20 approve(address,uint256) calldata: ${(e as Error).message}`, + expected, + decoded + ) + ); + return warnings; + } + + if (!ctx.batcherAddress) { + warnings.push( + violation( + ctx, + 'no batcher contract address is configured for this network; tokenApproval spender cannot be verified', + expected, + { ...decoded, to: spender, value: amount } + ) + ); + } else if (normalizeAddress(spender) !== normalizeAddress(ctx.batcherAddress)) { + warnings.push( + violation( + ctx, + 'tokenApproval spender is not the network batcher contract', + { ...expected, to: ctx.batcherAddress }, + { ...decoded, to: spender, value: amount } + ) + ); + } + if (!valuesEqual(amount, MAX_UINT256)) { + warnings.push( + violation( + ctx, + 'tokenApproval amount is not the maximum ERC-20 allowance', + { ...expected, value: MAX_UINT256 }, + { ...decoded, to: spender, value: amount } + ) + ); + } + return warnings; +} + +/** `fillNonce` handler — a 1-wei self-send to the wallet base address. */ +function verifyFillNonce(ctx: EvmTssIntentContext, tx: DecodedEvmTx): TssFinding[] { + const warnings: TssFinding[] = []; + const decoded = { to: tx.to, value: tx.value, data: tx.data }; + const expected = { to: ctx.baseAddress, value: '1', data: '0x' }; + if (!ctx.baseAddress) { + warnings.push(violation(ctx, 'fillNonce intent has no wallet base address to verify against', expected, decoded)); + return warnings; + } + if (normalizeAddress(tx.to) !== normalizeAddress(ctx.baseAddress)) { + warnings.push(violation(ctx, 'fillNonce transaction must send to the wallet base address', expected, decoded)); + } + if (!valuesEqual(tx.value, '1')) { + warnings.push(violation(ctx, 'fillNonce transaction must send exactly 1 wei', expected, decoded)); + } + if (!isEmptyCalldata(tx.data)) { + warnings.push(violation(ctx, 'fillNonce transaction must not carry calldata', expected, decoded)); + } + return warnings; +} + +function unhandledTypeViolation(ctx: EvmTssIntentContext, tx: DecodedEvmTx): TssFinding { + return violation( + ctx, + `no fail-closed verifier for intent type '${ctx.intentType}': transaction bytes were not compared against intent`, + {}, + { to: tx.to, value: tx.value, data: tx.data }, + 'unhandled' + ); +} + +/** + * A consolidation must pay the wallet base address: a native send to it, or an ERC-20 transfer + * to it that carries no native value. Without the value check, `transfer(baseAddress, 0)` + * calldata sent to an arbitrary contract would move the native balance there. + */ +export function verifyConsolidation(tx: DecodedEvmTx, baseAddress: string, severity: TssFindingSeverity): TssFinding[] { + const ctx = { intentType: 'consolidate' }; + const expected = { to: baseAddress }; + const decoded = { to: tx.to, value: tx.value }; + if (!normalizeCalldata(tx.data).startsWith(erc20TransferMethodId)) { + return normalizeAddress(tx.to) === normalizeAddress(baseAddress) + ? [] + : [ + violation( + ctx, + 'Consolidation transaction recipient does not match wallet base address', + expected, + decoded, + severity + ), + ]; + } + // For ERC-20 consolidations `to` is the token contract; the real recipient is in the calldata. + const findings: TssFinding[] = []; + const [innerRecipient] = getRawDecoded(['address', 'uint256'], getBufferedByteCode(erc20TransferMethodId, tx.data)); + const decodedRecipient = toAddressString(innerRecipient); + if (decodedRecipient !== normalizeAddress(baseAddress)) { + findings.push( + violation( + ctx, + 'Consolidation transaction recipient does not match wallet base address', + expected, + { ...decoded, to: decodedRecipient }, + severity + ) + ); + } + if (!valuesEqual(tx.value, '0')) { + findings.push( + violation( + ctx, + 'Consolidation token transfer must not carry native value', + { ...expected, value: '0' }, + decoded, + severity + ) + ); + } + return findings; +} + +/** + * Evaluate the decoded transaction against the intent. Returns every violated rule for the + * canonical intent type; an empty array means the bytes match the closed set of accepted + * shapes for that type. + */ +export function evaluateEvmTssIntent(ctx: EvmTssIntentContext, tx: DecodedEvmTx): TssFinding[] { + switch (ctx.intentType) { + case 'transfer': + return verifyPayment(ctx, tx); + case 'transferToken': + return verifyTransferToken(ctx, tx); + case 'tokenApproval': + return verifyTokenApproval(ctx, tx); + case 'fillNonce': + return verifyFillNonce(ctx, tx); + case 'consolidate': + // The base-address check in verifyTssTransaction is the handler for consolidations. + return []; + case 'defiApprove': + case 'defiDeposit': + case 'defiWithdraw': + case 'wrapApprove': + // Dedicated fail-closed calldata checks in verifyTssTransaction handle these. + return []; + default: + return [unhandledTypeViolation(ctx, tx)]; + } +} diff --git a/modules/sdk-api/src/bitgoAPI.ts b/modules/sdk-api/src/bitgoAPI.ts index 16bbb40b7d8..5eb419557bc 100644 --- a/modules/sdk-api/src/bitgoAPI.ts +++ b/modules/sdk-api/src/bitgoAPI.ts @@ -24,6 +24,7 @@ import { IRequestTracer, makeRandomKey, sanitizeLegacyPath, + TssVerificationWarningHandler, } from '@bitgo/sdk-core'; import * as sdkHmac from '@bitgo/sdk-hmac'; import { DefaultHmacAuthStrategy, type IHmacAuthStrategy } from '@bitgo/sdk-hmac'; @@ -283,10 +284,16 @@ export class BitGoAPI implements BitGoBase { private _requestIdPrefix?: string; private getAdditionalHeadersCb?: AdditionalHeadersCallback; protected _hmacAuthStrategy: IHmacAuthStrategy; + /** @inheritDoc */ + public readonly strictTssVerification: boolean; + /** @inheritDoc */ + public readonly onVerificationWarning?: TssVerificationWarningHandler; constructor(params: BitGoAPIOptions = {}) { this.getAdditionalHeadersCb = params.getAdditionalHeadersCb; this._requestIdPrefix = params.requestIdPrefix; + this.strictTssVerification = params.strictTssVerification ?? false; + this.onVerificationWarning = params.onVerificationWarning; this.cookiesPropagationEnabled = false; if ( !common.validateParams( diff --git a/modules/sdk-api/src/types.ts b/modules/sdk-api/src/types.ts index 8e51d6d8d1f..d1136b8af9d 100644 --- a/modules/sdk-api/src/types.ts +++ b/modules/sdk-api/src/types.ts @@ -1,4 +1,10 @@ -import { EncryptionVersion, EnvironmentName, IRequestTracer, V1Network } from '@bitgo/sdk-core'; +import { + EncryptionVersion, + EnvironmentName, + IRequestTracer, + TssVerificationWarningHandler, + V1Network, +} from '@bitgo/sdk-core'; import { type BitGoTlsPolicy } from './tlsPolicy'; import { ECPairInterface } from '@bitgo/utxo-lib'; import { type Agent } from 'http'; @@ -76,6 +82,16 @@ export interface BitGoAPIOptions { cookiesPropagationEnabled?: boolean; getAdditionalHeadersCb?: AdditionalHeadersCallback; requestIdPrefix?: string; + /** + * Client-side opt-in: fail-closed EVM TSS verification warnings become hard failures + * instead of `console.warn` + `onVerificationWarning`. Never read from server data. + */ + strictTssVerification?: boolean; + /** + * Sink for fail-closed EVM TSS verification warnings (decoded vs expected `to` / + * `value` / `data` for a transaction about to be signed). + */ + onVerificationWarning?: TssVerificationWarningHandler; evm?: { [key: string]: { baseUrl?: string; diff --git a/modules/sdk-coin-bsc/test/unit/tssBatcherVerification.ts b/modules/sdk-coin-bsc/test/unit/tssBatcherVerification.ts new file mode 100644 index 00000000000..e69e5596489 --- /dev/null +++ b/modules/sdk-coin-bsc/test/unit/tssBatcherVerification.ts @@ -0,0 +1,312 @@ +import 'should'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; +import { coins, ContractAddressDefinedToken, Erc7984Coin } from '@bitgo/statics'; +import { TransactionType, TssVerificationWarning, Wallet } from '@bitgo/sdk-core'; +import { optionalDeps } from '@bitgo/abstract-eth'; + +import { Tbsc } from '../../src'; +import { TransactionBuilder } from '../../src/lib'; + +/** + * Fail-closed EVM TSS verification for batcher-contract flows (CECHO-2460). + * + * Native batches (`batch(address[],uint256[])`) and token batches + * (`batchTransferFrom(address,address[],uint256[])`) must reach the network's batcher + * contract with the intent's recipients 1:1, and `tokenApproval` must be + * `approve(batcher, MAX)` on the intent token contract. tbsc is used because its statics + * network carries a real batcher contract address. + */ + +const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; +const attackerAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; +const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; +const MAX_UINT256 = ((1n << 256n) - 1n).toString(); + +const erc7984Pair = coins.get('hteth:cusdt') as Erc7984Coin; +const erc7984WrapperAddress = String(erc7984Pair.contractAddress); + +// Two distinct tbsc tokens, so a batch can be shown to move exactly one asset. +const tokenContractOf = (tokenName: string): string => { + const token = coins.get(tokenName); + if (!(token instanceof ContractAddressDefinedToken)) { + throw new Error(`${tokenName} has no contract address in statics`); + } + return String(token.contractAddress); +}; +const busdTokenName = 'tbsc:busd'; +const usd1TokenName = 'tbsc:usd1'; +const busdContractAddress = tokenContractOf(busdTokenName); + +const word = (value: bigint): string => value.toString(16).padStart(64, '0'); +const addrWord = (address: string): string => address.toLowerCase().replace(/^0x/, '').padStart(64, '0'); + +function encodeApprove(spender: string, amount: string): string { + return `0x095ea7b3${addrWord(spender)}${word(BigInt(amount))}`; +} + +function encodeWithAbi(signature: string, types: string[], values: unknown[]): string { + const selector = optionalDeps.ethAbi.methodID(signature, types); + const args = optionalDeps.ethAbi.rawEncode(types, values); + return '0x' + Buffer.concat([selector, args]).toString('hex'); +} + +function encodeBatch(addresses: string[], amounts: string[]): string { + return encodeWithAbi('batch', ['address[]', 'uint256[]'], [addresses, amounts]); +} + +function encodeBatchTransferFrom(token: string, addresses: string[], amounts: string[]): string { + return encodeWithAbi('batchTransferFrom', ['address', 'address[]', 'uint256[]'], [token, addresses, amounts]); +} + +async function buildTxHex(opts: { to: string; value?: string; data?: string }): Promise { + const builder = new TransactionBuilder(coins.get('tbsc')); + builder.type(opts.data && opts.data !== '0x' ? TransactionType.ContractCall : TransactionType.SingleSigSend); + builder.fee({ fee: '10', gasLimit: '21000' }); + builder.counter(1); + builder.contract(opts.to); + if (opts.data) { + builder.data(opts.data); + } + if (opts.value) { + builder.value(opts.value); + } + return (await builder.build()).toBroadcastFormat(); +} + +describe('EVM TSS fail-closed verification (batcher contract flows)', function () { + const batcherAddress = String( + (coins.get('tbsc').network as unknown as { batcherContractAddress: string }).batcherContractAddress + ); + + let bitgo: TestBitGoAPI; + let strictBitgo: TestBitGoAPI; + let warnings: TssVerificationWarning[]; + let coin: Tbsc; + let wallet: Wallet; + + beforeEach(function () { + warnings = []; + bitgo = TestBitGo.decorate(BitGoAPI, { + env: 'test', + onVerificationWarning: (warning) => warnings.push(warning), + }); + strictBitgo = TestBitGo.decorate(BitGoAPI, { + env: 'test', + strictTssVerification: true, + }); + bitgo.safeRegister('tbsc', Tbsc.createInstance); + bitgo.initializeTestVars(); + strictBitgo.safeRegister('tbsc', Tbsc.createInstance); + strictBitgo.initializeTestVars(); + coin = bitgo.coin('tbsc') as Tbsc; + wallet = new Wallet( + bitgo, + coin as any, + { + coinSpecific: { baseAddress: recipientAddress }, + } as any + ); + }); + + const verify = (txParams: Record, txHex: string, strict = false): Promise => { + const target = strict ? (strictBitgo.coin('tbsc') as Tbsc) : coin; + const w = strict + ? new Wallet(strictBitgo, target as any, { coinSpecific: { baseAddress: recipientAddress } } as any) + : wallet; + return (target as any).verifyTransaction({ + txParams, + txPrebuild: { txHex, coin: 'tbsc', walletId: 'w' }, + wallet: w, + verification: {}, + walletType: 'tss', + }); + }; + + it('pins the hardcoded batch selectors to their runtime-computed values', function () { + encodeBatch([recipientAddress], ['1']).slice(0, 10).should.equal('0xc00c4e9e'); + encodeBatchTransferFrom(tokenContractAddress, [recipientAddress], ['1']).slice(0, 10).should.equal('0x4885b254'); + }); + + describe('native batch', function () { + const recipients = [ + { address: recipientAddress, amount: '10' }, + { address: attackerAddress, amount: '20' }, + ]; + + it('verifies batch calldata against the batcher contract and the intent arrays', async function () { + const txHex = await buildTxHex({ + to: batcherAddress, + value: '30', + data: encodeBatch([recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients }, txHex)).should.equal(true); + warnings.should.have.length(0); + }); + + it('flags swapped inner recipients and inflated batch value', async function () { + const swappedHex = await buildTxHex({ + to: batcherAddress, + value: '30', + data: encodeBatch([attackerAddress, recipientAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients }, swappedHex)).should.equal(true); + warnings.some((w) => /entry 0 does not match/.test(w.reason)).should.equal(true); + await verify({ type: 'transfer', recipients }, swappedHex, true).should.be.rejectedWith(/entry 0 does not match/); + + warnings = []; + const inflatedHex = await buildTxHex({ + to: batcherAddress, + value: '999', + data: encodeBatch([recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients }, inflatedHex)).should.equal(true); + warnings.some((w) => /value does not match the sum/.test(w.reason)).should.equal(true); + }); + + it('flags a batch sent somewhere other than the network batcher contract', async function () { + const txHex = await buildTxHex({ + to: attackerAddress, + value: '30', + data: encodeBatch([recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients }, txHex)).should.equal(true); + warnings.some((w) => /does not match the network batcher contract/.test(w.reason)).should.equal(true); + }); + + it('flags batch-shaped sends whose calldata is not batch or batchTransferFrom', async function () { + const txHex = await buildTxHex({ + to: batcherAddress, + value: '30', + data: encodeWithAbi('transfer', ['address', 'uint256'], [recipientAddress, '30']), + }); + (await verify({ type: 'transfer', recipients }, txHex)).should.equal(true); + warnings.some((w) => /must use batch\(address\[\],uint256\[\]\)/.test(w.reason)).should.equal(true); + }); + }); + + describe('token batch', function () { + const recipients = [ + { address: recipientAddress, amount: '10', tokenName: 'hteth:cusdt' }, + { address: attackerAddress, amount: '20', tokenName: 'hteth:cusdt' }, + ]; + + it('verifies batchTransferFrom against the intent token contract with zero value', async function () { + const txHex = await buildTxHex({ + to: batcherAddress, + value: '0', + data: encodeBatchTransferFrom(erc7984WrapperAddress, [recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients }, txHex)).should.equal(true); + warnings.should.have.length(0); + }); + + it('flags a token batch whose token contract is not the intent token', async function () { + const txHex = await buildTxHex({ + to: batcherAddress, + value: '0', + data: encodeBatchTransferFrom(tokenContractAddress, [recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients }, txHex)).should.equal(true); + warnings.some((w) => /token contract does not match/.test(w.reason)).should.equal(true); + await verify({ type: 'transfer', recipients }, txHex, true).should.be.rejectedWith( + /token contract does not match/ + ); + }); + }); + + describe('batch asset binding (CECHO-2460)', function () { + const nativeRecipients = [ + { address: recipientAddress, amount: '10' }, + { address: attackerAddress, amount: '20' }, + ]; + const tokenRecipients = [ + { address: recipientAddress, amount: '10', tokenName: busdTokenName }, + { address: attackerAddress, amount: '20', tokenName: busdTokenName }, + ]; + + it('rejects native batch calldata for a token batch intent', async function () { + const txHex = await buildTxHex({ + to: batcherAddress, + value: '30', + data: encodeBatch([recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients: tokenRecipients }, txHex)).should.equal(true); + warnings.some((w) => /token batch payment must use batchTransferFrom/.test(w.reason)).should.equal(true); + await verify({ type: 'transfer', recipients: tokenRecipients }, txHex, true).should.be.rejectedWith( + /token batch payment must use batchTransferFrom/ + ); + }); + + it('rejects batchTransferFrom calldata for a native batch intent', async function () { + const txHex = await buildTxHex({ + to: batcherAddress, + value: '0', + data: encodeBatchTransferFrom(busdContractAddress, [recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients: nativeRecipients }, txHex)).should.equal(true); + warnings + .some((w) => /native batch payment must use batch\(address\[\],uint256\[\]\)/.test(w.reason)) + .should.equal(true); + await verify({ type: 'transfer', recipients: nativeRecipients }, txHex, true).should.be.rejectedWith( + /native batch payment must use batch\(address\[\],uint256\[\]\)/ + ); + }); + + it('rejects a batch whose recipients declare two different tokens', async function () { + const mixedRecipients = [ + { address: recipientAddress, amount: '10', tokenName: busdTokenName }, + { address: attackerAddress, amount: '20', tokenName: usd1TokenName }, + ]; + const txHex = await buildTxHex({ + to: batcherAddress, + value: '0', + data: encodeBatchTransferFrom(busdContractAddress, [recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients: mixedRecipients }, txHex)).should.equal(true); + warnings.some((w) => /more than one asset/.test(w.reason)).should.equal(true); + await verify({ type: 'transfer', recipients: mixedRecipients }, txHex, true).should.be.rejectedWith( + /more than one asset/ + ); + }); + + it('rejects a batch with an unresolvable token identity', async function () { + const unresolvedRecipients = [ + { address: recipientAddress, amount: '10', tokenName: 'tbsc:not-a-registered-token' }, + { address: attackerAddress, amount: '20', tokenName: 'tbsc:not-a-registered-token' }, + ]; + const txHex = await buildTxHex({ + to: batcherAddress, + value: '0', + data: encodeBatchTransferFrom(busdContractAddress, [recipientAddress, attackerAddress], ['10', '20']), + }); + (await verify({ type: 'transfer', recipients: unresolvedRecipients }, txHex)).should.equal(true); + warnings.some((w) => /cannot be resolved to a token contract/.test(w.reason)).should.equal(true); + }); + }); + + describe('tokenApproval', function () { + const approvalParams = { type: 'tokenApproval', tokenName: 'hteth:cusdt' }; + + it('verifies approve(batcher, MAX) on the intent token contract', async function () { + const txHex = await buildTxHex({ + to: erc7984WrapperAddress, + value: '0', + data: encodeApprove(batcherAddress, MAX_UINT256), + }); + (await verify(approvalParams, txHex)).should.equal(true); + warnings.should.have.length(0); + }); + + it('flags approve(attacker, MAX) under a tokenApproval intent (TOB-BITGOEDMPC-2)', async function () { + const txHex = await buildTxHex({ + to: erc7984WrapperAddress, + value: '0', + data: encodeApprove(attackerAddress, MAX_UINT256), + }); + (await verify(approvalParams, txHex)).should.equal(true); + warnings.some((w) => /spender is not the network batcher contract/.test(w.reason)).should.equal(true); + await verify(approvalParams, txHex, true).should.be.rejectedWith(/spender is not the network batcher contract/); + }); + }); +}); diff --git a/modules/sdk-coin-eth/test/unit/eth.ts b/modules/sdk-coin-eth/test/unit/eth.ts index 9eeb9106d37..aa9df0ed083 100644 --- a/modules/sdk-coin-eth/test/unit/eth.ts +++ b/modules/sdk-coin-eth/test/unit/eth.ts @@ -1626,7 +1626,7 @@ describe('ETH:', function () { }); }; - ['tokenApproval', 'customTx', 'defiApprove'].forEach((type) => { + ['tokenApproval', 'acceleration', '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,62 +1635,44 @@ describe('ETH:', function () { }); }); - it('should reject a mutated-amount wrapper approve relabelled as customTx', async function () { + it('should reject customTx and contractCall outright as unsupported EVM TSS intent types', async function () { + for (const type of ['customTx', 'contractCall']) { + await verifyAs(type, defiCalldata('approve(address,uint256)', [wrapper, amount])).should.be.rejectedWith( + /unsupported intent type for EVM TSS/ + ); + } + }); + + it('should reject a mutated-amount wrapper approve relabelled as another type', async function () { await expectDefiRejection( - verifyAs('customTx', defiCalldata('approve(address,uint256)', [wrapper, '999999999999'])), + verifyAs('acceleration', 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('acceleration', 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('acceleration', 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 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. 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], + ['bridgeFunds', defiCalldata('transfer(address,uint256)', [address2, amount]), underlying], + ['bridgeFunds', defiCalldata('approve(address,uint256)', [address2, amount]), underlying], + ['bridgeFunds', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2], ['tokenApproval', defiCalldata('approve(address,uint256)', [address2, amount]), underlying], ['tokenApproval', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2], ['acceleration', defiCalldata('transfer(address,uint256)', [address2, amount]), address2], @@ -1703,9 +1685,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('acceleration', defiCalldata('transfer(address,uint256)', [wrapper, amount]))).should.equal( + true + ); ( - await verifyAs('customTx', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2) + await verifyAs('acceleration', defiCalldata('approve(address,uint256)', [wrapper, amount]), address2) ).should.equal(true); }); 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/tssSigningPath.ts b/modules/sdk-coin-evm/test/unit/tssSigningPath.ts new file mode 100644 index 00000000000..148d16fff3f --- /dev/null +++ b/modules/sdk-coin-evm/test/unit/tssSigningPath.ts @@ -0,0 +1,125 @@ +import 'should'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGo } from '@bitgo/sdk-test'; +import { coins } from '@bitgo/statics'; +import { + EcdsaUtils, + RequestTracer, + TransactionType, + TxIntentMismatchRecipientError, + TxRequest, + Wallet, +} from '@bitgo/sdk-core'; + +import { EvmCoin } from '../../src/evmCoin'; +import { TransactionBuilder } from '../../src/lib'; + +/** + * End to end on the ECDSA signing path (CECHO-2460). tssVerification.ts calls verifyTransaction + * directly; this suite runs the real EVM verifier inside EcdsaUtils.signTxRequest and proves that + * bytes it rejects never reach the signing round. + */ +describe('EVM TSS signing path stops tampered bytes before signing', function () { + const chain = 'kavaevm'; + const staticsCoin = coins.get(chain); + const walletId = '62fe536a6b4cf70007acb48c0e7bb0b0'; + const baseAddress = '0x1111111111111111111111111111111111111111'; + const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; + const attackerAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; + const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; + 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: {} }); + const paymentIntent = { + intentType: 'payment', + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: chain } }], + }; + const callerPayment = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + + const buildTxHex = async (opts: { to: string; value: string; data?: string }): Promise => { + const builder = new TransactionBuilder(staticsCoin); + builder.type(opts.data ? TransactionType.ContractCall : TransactionType.SingleSigSend); + builder.fee({ fee: '10', gasLimit: '21000' }); + builder.counter(1); + builder.contract(opts.to); + if (opts.data) { + builder.data(opts.data); + } + builder.value(opts.value); + return (await builder.build()).toBroadcastFormat(); + }; + + /** Signs through EcdsaUtils and reports whether the signing round (after verification) started. */ + const sign = async (opts: { + txHex: string; + txParams?: Record; + strict?: boolean; + }): Promise<{ signingStarted: boolean; error?: Error }> => { + const bitgo = TestBitGo.decorate(BitGoAPI, { + env: 'test', + strictTssVerification: opts.strict ?? false, + onVerificationWarning: () => undefined, + }); + const coin = new (EvmCoin as any)(bitgo, staticsCoin); + const wallet = new Wallet(bitgo, coin, { id: walletId, coinSpecific: { baseAddress }, multisigType: 'tss' } as any); + const utils = new EcdsaUtils(bitgo, coin, wallet); + let signingStarted = false; + (utils as any).getEcdsaSigningChallenges = async () => { + signingStarted = true; + throw new Error('stop: signing round started'); + }; + const txRequest = { + txRequestId: 'signing-path', + apiVersion: 'full', + walletId, + intent: paymentIntent, + transactions: [ + { + unsignedTx: { derivationPath: 'm/0', signableHex: opts.txHex, serializedTxHex: opts.txHex }, + signatureShares: [], + }, + ], + } as unknown as TxRequest; + try { + await utils.signTxRequest({ txRequest, txParams: opts.txParams as any, prv, reqId: new RequestTracer() }); + } catch (e) { + return { signingStarted, error: e as Error }; + } + return { signingStarted }; + }; + + it('reaches the signing round for a payment whose bytes match the intent', async function () { + const txHex = await buildTxHex({ to: recipientAddress, value: '1000' }); + for (const txParams of [undefined, callerPayment]) { + const { signingStarted, error } = await sign({ txHex, txParams, strict: true }); + signingStarted.should.equal(true); + (error as Error).message.should.match(/signing round started/); + } + }); + + it('never starts signing a redirected payment on the pending-approval path, in either mode', async function () { + const txHex = await buildTxHex({ to: attackerAddress, value: '1000' }); + for (const strict of [false, true]) { + const { signingStarted, error } = await sign({ txHex, strict }); + signingStarted.should.equal(false); + (error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (error as Error).message.should.match(/destination address does not match/); + } + }); + + it('never starts signing approve(attacker, MAX) labelled as a payment under strictTssVerification', async function () { + const txHex = await buildTxHex({ to: tokenContractAddress, value: '0', data: approveAttackerMax }); + const { signingStarted, error } = await sign({ txHex, txParams: callerPayment, strict: true }); + signingStarted.should.equal(false); + (error as Error).should.be.instanceOf(TxIntentMismatchRecipientError); + (error as Error).message.should.match(/not a recognized payment shape/); + }); + + it('lets the same bytes reach signing in default mode, as the warn-first rollout agreed', async function () { + // Pins today's policy so a change to it is deliberate: this flips once strict becomes the default. + const txHex = await buildTxHex({ to: tokenContractAddress, value: '0', data: approveAttackerMax }); + const { signingStarted } = await sign({ txHex, txParams: callerPayment }); + signingStarted.should.equal(true); + }); +}); diff --git a/modules/sdk-coin-evm/test/unit/tssVerification.ts b/modules/sdk-coin-evm/test/unit/tssVerification.ts new file mode 100644 index 00000000000..06e89649434 --- /dev/null +++ b/modules/sdk-coin-evm/test/unit/tssVerification.ts @@ -0,0 +1,913 @@ +import 'should'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; +import { BaseCoin as StaticsBaseCoin, coins, ContractAddressDefinedToken, Erc7984Coin } from '@bitgo/statics'; +import { + PopulatedIntent, + resolveEffectiveTxParams, + TransactionParams, + TransactionType, + TssVerificationWarning, + TxRequest, + Wallet, +} from '@bitgo/sdk-core'; +import { optionalDeps } from '@bitgo/abstract-eth'; + +import { EvmCoin } from '../../src/evmCoin'; +import { TransactionBuilder } from '../../src/lib'; + +/** + * Fail-closed EVM TSS verification (TOB-BITGOEDMPC-2 / INFOSEC-671 / CECHO-2460). + * + * The user's key share must not sign EVM TSS bytes unless the decoded to / value / data match + * the intent. Checks that threw before this change keep throwing (including on the + * pending-approval path); every new check flags the violation (warn by default, throw under + * `strictTssVerification`). + * + * Runs on kavaevm (legacy, non-EIP-1559 — the chains that used to bypass the verifier through + * `EvmCoin.verifyLegacyTssTransaction`) and hbarevm (EIP-1559). Batcher-contract cases + * (native/token batch sends, tokenApproval spender) need a chain with a statics batcher + * address and live in @bitgo/sdk-coin-bsc's tssBatcherVerification tests. + */ + +const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be'; +const attackerAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6'; +const baseAddress = '0x1111111111111111111111111111111111111111'; +const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48'; +const MAX_UINT256 = ((1n << 256n) - 1n).toString(); + +const erc7984Pair = coins.get('hteth:cusdt') as Erc7984Coin; +const erc7984WrapperAddress = String(erc7984Pair.contractAddress); + +// Two distinct registered tokens whose contracts resolve locally from statics. The caller +// and the stored intent name different tokens to prove the server cannot replace local identity. +const tokenContractOf = (tokenName: string): string => { + const token = coins.get(tokenName); + if (!(token instanceof ContractAddressDefinedToken)) { + throw new Error(`${tokenName} has no contract address in statics`); + } + return String(token.contractAddress); +}; +const localTokenName = 'hteth:tusdc'; +const serverTokenName = 'hteth:tusdt'; +const localTokenContractAddress = tokenContractOf(localTokenName); +const serverTokenContractAddress = tokenContractOf(serverTokenName); + +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 encodeApprove(spender: string, amount: string): string { + return `0x095ea7b3${addrWord(spender)}${word(BigInt(amount))}`; +} + +function encodeWithAbi(signature: string, types: string[], values: unknown[]): string { + const selector = optionalDeps.ethAbi.methodID(signature, types); + const args = optionalDeps.ethAbi.rawEncode(types, values); + return '0x' + Buffer.concat([selector, args]).toString('hex'); +} + +function encodeConfidentialTransferNoProof(to: string): string { + return encodeWithAbi('confidentialTransfer', ['address', 'bytes32'], [to, '0x' + '11'.repeat(32)]); +} + +async function buildTxHex( + coin: Readonly, + opts: { to: string; value?: string; data?: string } +): Promise { + const builder = new TransactionBuilder(coin); + builder.type(opts.data && opts.data !== '0x' ? TransactionType.ContractCall : TransactionType.SingleSigSend); + builder.fee({ fee: '10', gasLimit: '21000' }); + builder.counter(1); + builder.contract(opts.to); + if (opts.data) { + builder.data(opts.data); + } + if (opts.value) { + builder.value(opts.value); + } + return (await builder.build()).toBroadcastFormat(); +} + +// kavaevm is legacy (non-EIP-1559) and hbarevm is EIP-1559; neither has a statics batcher +// contract, so the "statics gap" paths are what both exercise for batcher-dependent rules. +['kavaevm', 'hbarevm'].forEach((chain) => { + describe(`EVM TSS fail-closed verification (${chain})`, function () { + const staticsCoin = coins.get(chain); + + let bitgo: TestBitGoAPI; + let strictBitgo: TestBitGoAPI; + let warnings: TssVerificationWarning[]; + let coin: EvmCoin; + let wallet: Wallet; + + beforeEach(function () { + warnings = []; + bitgo = TestBitGo.decorate(BitGoAPI, { + env: 'test', + onVerificationWarning: (warning) => warnings.push(warning), + }); + strictBitgo = TestBitGo.decorate(BitGoAPI, { + env: 'test', + strictTssVerification: true, + }); + coin = new (EvmCoin as any)(bitgo, staticsCoin); + wallet = new Wallet(bitgo, coin as any, { coinSpecific: { baseAddress } } as any); + }); + + // Caller params may also carry the EVM-only tokenName that EthTransactionParams adds. + type SigningPathCallerParams = TransactionParams & { tokenName?: string }; + + const verify = ( + txParams: Record, + txHex: string, + opts: { + strict?: boolean; + intent?: PopulatedIntent; + callerTxParams?: SigningPathCallerParams; + verification?: Record; + } = {} + ): Promise => { + const target = opts.strict ? new (EvmCoin as any)(strictBitgo, staticsCoin) : coin; + const w = opts.strict ? new Wallet(strictBitgo, target as any, { coinSpecific: { baseAddress } } as any) : wallet; + return (target as any).verifyTransaction({ + txParams, + txPrebuild: { txHex, coin: chain, walletId: 'w' }, + wallet: w, + verification: opts.verification ?? {}, + walletType: 'tss', + ...(opts.intent ? { intent: opts.intent } : {}), + ...(opts.callerTxParams ? { callerTxParams: opts.callerTxParams } : {}), + }); + }; + + // Mirrors the ECDSA / MPCv2 signing paths: resolveEffectiveTxParams back-fills the stored + // intent into the effective txParams, while callerTxParams stays the untouched caller + // input. This is the wiring that decides which source speaks for the user intent. + const verifySigningPath = ( + callerTxParams: SigningPathCallerParams | undefined, + intent: Record, + txHex: string, + strict = false + ): Promise => { + const txRequest = { txRequestId: 'signing-path', walletId: 'w', intent } as unknown as TxRequest; + const effective = resolveEffectiveTxParams(txRequest, callerTxParams, chain); + return verify(effective as unknown as Record, txHex, { + strict, + intent: intent as unknown as PopulatedIntent, + ...(callerTxParams ? { callerTxParams } : {}), + }); + }; + + const build = (opts: { to: string; value?: string; data?: string }): Promise => + buildTxHex(staticsCoin, opts); + + describe('payment handler (transfer)', function () { + it('verifies a native send that matches the intent, with no warnings', async function () { + const txHex = await build({ to: recipientAddress, value: '1000' }); + const result = await verify( + { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, + txHex + ); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('throws on a redirected destination (existing check, both modes)', async function () { + const txHex = await build({ to: attackerAddress, value: '1000' }); + for (const strict of [false, true]) { + await verify({ type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, txHex, { + strict, + }).should.be.rejectedWith(/destination address does not match/); + } + }); + + it('throws on an inflated value (existing check, both modes)', async function () { + const txHex = await build({ to: recipientAddress, value: '9999' }); + for (const strict of [false, true]) { + await verify({ type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, txHex, { + strict, + }).should.be.rejectedWith(/amount in txPrebuild does not match/); + } + }); + + it('verifies an exact dApp call (to, value and data equal to the intent)', async function () { + const data = '0xdeadbeef'; + const txHex = await build({ to: tokenContractAddress, value: '0', data }); + const result = await verify( + { type: 'transfer', recipients: [{ address: tokenContractAddress, amount: '0', data }] }, + txHex + ); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('flags swapped calldata under a payment intent (warn, throw in strict mode)', async function () { + // ERC-20 calldata with a swapped inner recipient: the existing inner check throws. + const intentData = encodeErc20Transfer(recipientAddress, '1000'); + const tamperedData = encodeErc20Transfer(attackerAddress, '1000'); + const tamperedHex = await build({ to: tokenContractAddress, value: '0', data: tamperedData }); + await verify( + { + type: 'transfer', + recipients: [{ address: tokenContractAddress, amount: '0', data: intentData }], + }, + tamperedHex + ).should.be.rejectedWith(/destination address does not match/); + + // Non-ERC-20 dApp calldata mismatch is a new check: warn, or throw in strict mode. + const swappedHex = await build({ to: tokenContractAddress, value: '0', data: '0xdeadbeef' }); + const txParams = { + type: 'transfer', + recipients: [{ address: tokenContractAddress, amount: '0', data: '0xdeadbe00' }], + }; + const result = await verify(txParams, swappedHex); + result.should.equal(true); + warnings.should.have.length(1); + warnings[0].reason.should.match(/calldata does not match/); + warnings[0].decoded.data.should.equal('0xdeadbeef'); + warnings[0].expected.data.should.equal('0xdeadbe00'); + await verify(txParams, swappedHex, { strict: true }).should.be.rejectedWith(/calldata does not match/); + }); + + it('flags a mismatched destination under an exact-call (dApp) intent', async function () { + const data = '0xdeadbeef'; + const txHex = await build({ to: attackerAddress, value: '0', data }); + const txParams = { + type: 'transfer', + recipients: [{ address: tokenContractAddress, amount: '0', data }], + }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /to does not match/.test(w.reason)).should.equal(true); + await verify(txParams, txHex, { strict: true }).should.be.rejectedWith(/to does not match/); + }); + + it('flags a mismatched value under an exact-call (dApp) intent', async function () { + const data = '0xdeadbeef'; + const txHex = await build({ to: tokenContractAddress, value: '7', data }); + const txParams = { + type: 'transfer', + recipients: [{ address: tokenContractAddress, amount: '0', data }], + }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /value does not match/.test(w.reason)).should.equal(true); + await verify(txParams, txHex, { strict: true }).should.be.rejectedWith(/value does not match/); + }); + + it('flags approve(attacker, MAX) disguised as a transfer (TOB-BITGOEDMPC-2)', async function () { + const txHex = await build({ + to: tokenContractAddress, + value: '0', + data: encodeApprove(attackerAddress, MAX_UINT256), + }); + const txParams = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /not a recognized payment shape/.test(w.reason)).should.equal(true); + await verify(txParams, txHex, { strict: true }).should.be.rejectedWith(/not a recognized payment shape/); + }); + + it('flags missing recipients under a payment intent', async function () { + const txHex = await build({ to: recipientAddress, value: '1000' }); + const txParams = { type: 'transfer', recipients: [] }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /no recipients/.test(w.reason)).should.equal(true); + await verify(txParams, txHex, { strict: true }).should.be.rejectedWith(/no recipients/); + }); + }); + + describe('payment handler (token transfers)', function () { + const tokenRecipient = { + address: recipientAddress, + amount: '10000000', + tokenAddress: tokenContractAddress, + }; + + it('verifies an ERC-20 transfer against the intent token contract, value zero', async function () { + const txHex = await build({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '10000000'), + }); + const result = await verify({ type: 'transfer', recipients: [tokenRecipient] }, txHex); + result.should.equal(true); + warnings.should.have.length(0); + }); + + it('flags an ERC-20 transfer whose to is not the intent token contract', async function () { + const txHex = await build({ + to: attackerAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '10000000'), + }); + const txParams = { type: 'transfer', recipients: [tokenRecipient] }; + (await verify(txParams, txHex)).should.equal(true); + const flag = warnings.find((w) => /not the intent token contract/.test(w.reason)); + should.exist(flag); + flag?.expected.to?.toLowerCase().should.equal(tokenContractAddress.toLowerCase()); + await verify(txParams, txHex, { strict: true }).should.be.rejectedWith(/not the intent token contract/); + }); + + it('flags an ERC-20 transfer that carries native value', async function () { + const txHex = await build({ + to: tokenContractAddress, + value: '5', + data: encodeErc20Transfer(recipientAddress, '10000000'), + }); + const txParams = { type: 'transfer', recipients: [tokenRecipient] }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /must not carry native value/.test(w.reason)).should.equal(true); + await verify(txParams, txHex, { strict: true }).should.be.rejectedWith(/must not carry native value/); + }); + + it('throws on a tampered ERC-20 recipient or amount (existing checks)', async function () { + const toAttacker = await build({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(attackerAddress, '10000000'), + }); + await verify({ type: 'transfer', recipients: [tokenRecipient] }, toAttacker).should.be.rejectedWith( + /destination address does not match/ + ); + const inflated = await build({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '99999999'), + }); + await verify({ type: 'transfer', recipients: [tokenRecipient] }, inflated).should.be.rejectedWith( + /amount in txPrebuild does not match/ + ); + }); + + it('checks the ERC-7984 confidentialTransfer recipient only', async function () { + const intentParams = { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '0', tokenName: 'hteth:cusdt' }], + }; + const goodHex = await build({ + to: erc7984WrapperAddress, + value: '0', + data: encodeConfidentialTransferNoProof(recipientAddress), + }); + (await verify(intentParams, goodHex)).should.equal(true); + warnings.should.have.length(0); + + const tamperedHex = await build({ + to: erc7984WrapperAddress, + value: '0', + data: encodeConfidentialTransferNoProof(attackerAddress), + }); + (await verify(intentParams, tamperedHex)).should.equal(true); + warnings.some((w) => /confidentialTransfer recipient does not match/.test(w.reason)).should.equal(true); + await verify(intentParams, tamperedHex, { strict: true }).should.be.rejectedWith( + /confidentialTransfer recipient does not match/ + ); + }); + + it('flags batch calldata carried by a single declared recipient', async function () { + const txHex = await build({ + to: tokenContractAddress, + value: '0', + data: encodeWithAbi('batch', ['address[]', 'uint256[]'], [[recipientAddress], ['1']]), + }); + const txParams = { type: 'transfer', recipients: [tokenRecipient] }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /single declared recipient/.test(w.reason)).should.equal(true); + }); + }); + + describe('declared intent isolation and asset binding (CECHO-2460)', function () { + const tokenTransferData = encodeErc20Transfer(recipientAddress, '1000'); + const approvalData = encodeApprove(attackerAddress, MAX_UINT256); + + it('does not let stored calldata fill a caller recipient that declared none', async function () { + // The caller declares a native payment and supplies no calldata. The stored intent adds + // approve(attacker, MAX): it must not turn the caller's payment into an exact dApp call. + const caller = { type: 'transfer', recipients: [{ address: tokenContractAddress, amount: '0' }] }; + const intent = { + intentType: 'payment', + recipients: [ + { + address: { address: tokenContractAddress }, + amount: { value: '0', symbol: 'hteth' }, + data: approvalData, + }, + ], + }; + const txHex = await build({ to: tokenContractAddress, value: '0', data: approvalData }); + + (await verifySigningPath(caller, intent, txHex)).should.equal(true); + warnings.some((w) => /not a recognized payment shape/.test(w.reason)).should.equal(true); + await verifySigningPath(caller, intent, txHex, true).should.be.rejectedWith(/not a recognized payment shape/); + }); + + it('keeps the caller token identity when the stored intent names another token', async function () { + const caller = { + type: 'transferToken', + tokenName: localTokenName, + recipients: [{ address: recipientAddress, amount: '1000' }], + }; + const intent = { + intentType: 'transferToken', + tokenName: serverTokenName, + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: serverTokenName } }], + }; + const txHex = await build({ to: serverTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(caller, intent, txHex)).should.equal(true); + warnings.some((w) => /not the intent token contract/.test(w.reason)).should.equal(true); + await verifySigningPath(caller, intent, txHex, true).should.be.rejectedWith(/not the intent token contract/); + }); + + it('rejects token calldata for a caller-declared native payment', async function () { + const caller = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + const intent = { + intentType: 'payment', + recipients: [ + { + address: { address: recipientAddress }, + amount: { value: '1000', symbol: 'hteth' }, + tokenData: { tokenName: localTokenName }, + }, + ], + }; + const txHex = await build({ to: localTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(caller, intent, txHex)).should.equal(true); + warnings + .some((w) => /native asset but the transaction carries token or batch calldata/.test(w.reason)) + .should.equal(true); + await verifySigningPath(caller, intent, txHex, true).should.be.rejectedWith( + /native asset but the transaction carries token or batch calldata/ + ); + }); + + it('rejects a native send for a caller-declared token payment', async function () { + const caller = { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '1000', tokenAddress: localTokenContractAddress }], + }; + const intent = { + intentType: 'payment', + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: localTokenName } }], + }; + const txHex = await build({ to: recipientAddress, value: '1000' }); + + (await verifySigningPath(caller, intent, txHex)).should.equal(true); + warnings + .some((w) => /token payment intent but the transaction is a native send/.test(w.reason)) + .should.equal(true); + await verifySigningPath(caller, intent, txHex, true).should.be.rejectedWith( + /token payment intent but the transaction is a native send/ + ); + }); + + it('flags a tokenAddress that does not match the transaction token', async function () { + const caller = { + type: 'transfer', + recipients: [{ address: recipientAddress, amount: '1000', tokenAddress: localTokenContractAddress }], + }; + const txHex = await build({ to: serverTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(caller, { intentType: 'payment', recipients: [] }, txHex)).should.equal(true); + warnings.some((w) => /not the intent token contract/.test(w.reason)).should.equal(true); + await verifySigningPath(caller, { intentType: 'payment', recipients: [] }, txHex, true).should.be.rejectedWith( + /not the intent token contract/ + ); + }); + + it('flags an unresolvable token name instead of silently skipping the token check', async function () { + const caller = { + type: 'transferToken', + tokenName: 'hteth:not-a-registered-token', + recipients: [{ address: recipientAddress, amount: '1000' }], + }; + const txHex = await build({ to: localTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(caller, { intentType: 'transferToken', recipients: [] }, txHex)).should.equal(true); + warnings.some((w) => /cannot be resolved to a token contract/.test(w.reason)).should.equal(true); + await verifySigningPath( + caller, + { intentType: 'transferToken', recipients: [] }, + txHex, + true + ).should.be.rejectedWith(/cannot be resolved to a token contract/); + }); + + it('flags conflicting local token identity (tokenAddress and tokenName name two contracts)', async function () { + const caller = { + type: 'transferToken', + tokenName: serverTokenName, + recipients: [{ address: recipientAddress, amount: '1000', tokenAddress: localTokenContractAddress }], + }; + const txHex = await build({ to: localTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(caller, { intentType: 'transferToken', recipients: [] }, txHex)).should.equal(true); + warnings.some((w) => /name different token contracts/.test(w.reason)).should.equal(true); + await verifySigningPath( + caller, + { intentType: 'transferToken', recipients: [] }, + txHex, + true + ).should.be.rejectedWith(/name different token contracts/); + }); + + it('verifies a legitimate caller payment even when the stored intent contradicts it', async function () { + const caller = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + const intent = { + intentType: 'payment', + recipients: [ + { + address: { address: recipientAddress }, + amount: { value: '1000', symbol: 'hteth' }, + data: approvalData, + }, + ], + }; + const txHex = await build({ to: recipientAddress, value: '1000' }); + + (await verifySigningPath(caller, intent, txHex)).should.equal(true); + warnings.should.have.length(0); + (await verifySigningPath(caller, intent, txHex, true)).should.equal(true); + }); + + it('verifies a legitimate caller token payment even when the stored intent names another token', async function () { + const caller = { + type: 'transferToken', + tokenName: localTokenName, + recipients: [{ address: recipientAddress, amount: '1000' }], + }; + const intent = { + intentType: 'transferToken', + tokenName: serverTokenName, + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: serverTokenName } }], + }; + const txHex = await build({ to: localTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(caller, intent, txHex)).should.equal(true); + warnings.should.have.length(0); + }); + + it('uses the stored token identity when the caller declared nothing (pending approval)', async function () { + const intent = { + intentType: 'transferToken', + tokenName: localTokenName, + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: localTokenName } }], + }; + const txHex = await build({ to: localTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(undefined, intent, txHex)).should.equal(true); + warnings.should.have.length(0); + }); + + it('fails closed on an unresolvable stored token identity (pending approval)', async function () { + const intent = { + intentType: 'transferToken', + tokenName: 'hteth:not-a-registered-token', + recipients: [ + { + address: { address: recipientAddress }, + amount: { value: '1000', symbol: 'hteth:not-a-registered-token' }, + }, + ], + }; + const txHex = await build({ to: localTokenContractAddress, value: '0', data: tokenTransferData }); + + (await verifySigningPath(undefined, intent, txHex)).should.equal(true); + warnings.some((w) => /cannot be resolved to a token contract/.test(w.reason)).should.equal(true); + await verifySigningPath(undefined, intent, txHex, true).should.be.rejectedWith( + /cannot be resolved to a token contract/ + ); + }); + }); + + describe('batch payments without a configured batcher contract (statics gap)', function () { + const recipients = [ + { address: recipientAddress, amount: '10' }, + { address: attackerAddress, amount: '20' }, + ]; + + it('flags native batch calldata instead of silently passing it', async function () { + const txHex = await build({ + to: attackerAddress, + value: '30', + data: encodeWithAbi( + 'batch', + ['address[]', 'uint256[]'], + [ + [recipientAddress, attackerAddress], + ['10', '20'], + ] + ), + }); + const txParams = { type: 'transfer', recipients }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /no batcher contract address is configured/.test(w.reason)).should.equal(true); + await verify(txParams, txHex, { strict: true }).should.be.rejectedWith( + /no batcher contract address is configured/ + ); + }); + + it('flags token batch calldata instead of silently passing it', async function () { + const txHex = await build({ + to: attackerAddress, + value: '0', + data: encodeWithAbi( + 'batchTransferFrom', + ['address', 'address[]', 'uint256[]'], + [tokenContractAddress, [recipientAddress, attackerAddress], ['10', '20']] + ), + }); + (await verify({ type: 'transfer', recipients }, txHex)).should.equal(true); + warnings.some((w) => /no batcher contract address is configured/.test(w.reason)).should.equal(true); + }); + }); + + describe('tokenApproval handler', function () { + const approvalParams = { type: 'tokenApproval', tokenName: 'hteth:cusdt' }; + + it('flags a tokenApproval when the network has no configured batcher contract (statics gap)', async function () { + const txHex = await build({ + to: erc7984WrapperAddress, + value: '0', + data: encodeApprove(attackerAddress, MAX_UINT256), + }); + (await verify(approvalParams, txHex)).should.equal(true); + warnings.some((w) => /no batcher contract address is configured/.test(w.reason)).should.equal(true); + }); + + it('flags a non-maximum allowance, non-approve calldata, native value and a wrong to', async function () { + const smallAllowance = await build({ + to: erc7984WrapperAddress, + value: '0', + data: encodeApprove(attackerAddress, '5'), + }); + (await verify(approvalParams, smallAllowance)).should.equal(true); + warnings.some((w) => /not the maximum ERC-20 allowance/.test(w.reason)).should.equal(true); + + warnings = []; + const notApprove = await build({ + to: erc7984WrapperAddress, + value: '0', + data: encodeErc20Transfer(recipientAddress, '5'), + }); + (await verify(approvalParams, notApprove)).should.equal(true); + warnings.some((w) => /must use ERC-20 approve/.test(w.reason)).should.equal(true); + + warnings = []; + const carriesValue = await build({ + to: erc7984WrapperAddress, + value: '1', + data: encodeApprove(attackerAddress, MAX_UINT256), + }); + (await verify(approvalParams, carriesValue)).should.equal(true); + warnings.some((w) => /must not carry native value/.test(w.reason)).should.equal(true); + + warnings = []; + const wrongTo = await build({ + to: attackerAddress, + value: '0', + data: encodeApprove(attackerAddress, MAX_UINT256), + }); + (await verify(approvalParams, wrongTo)).should.equal(true); + warnings.some((w) => /to is not the intent token contract/.test(w.reason)).should.equal(true); + }); + }); + + describe('fillNonce handler', function () { + it('verifies a 1-wei self-send to the base address and flags deviations', async function () { + const good = await build({ to: baseAddress, value: '1' }); + (await verify({ type: 'fillNonce' }, good)).should.equal(true); + warnings.should.have.length(0); + + const wrongTo = await build({ to: recipientAddress, value: '1' }); + (await verify({ type: 'fillNonce' }, wrongTo)).should.equal(true); + warnings.some((w) => /must send to the wallet base address/.test(w.reason)).should.equal(true); + + warnings = []; + const wrongValue = await build({ to: baseAddress, value: '2' }); + (await verify({ type: 'fillNonce' }, wrongValue)).should.equal(true); + warnings.some((w) => /must send exactly 1 wei/.test(w.reason)).should.equal(true); + + warnings = []; + const withCalldata = await build({ to: baseAddress, value: '1', data: '0xdeadbeef' }); + (await verify({ type: 'fillNonce' }, withCalldata)).should.equal(true); + warnings.some((w) => /must not carry calldata/.test(w.reason)).should.equal(true); + + await verify({ type: 'fillNonce' }, wrongTo, { strict: true }).should.be.rejectedWith( + /must send to the wallet base address/ + ); + }); + }); + + describe('consolidation to the wallet base address', function () { + const consolidate = { type: 'consolidate', recipients: [] }; + const toBase = { consolidationToBaseAddress: true }; + + it('verifies a token consolidation that pays the base address and carries no native value', async function () { + const txHex = await build({ + to: tokenContractAddress, + value: '0', + data: encodeErc20Transfer(baseAddress, '5'), + }); + (await verify(consolidate, txHex, { verification: toBase })).should.equal(true); + (await verify(consolidate, txHex)).should.equal(true); + warnings.should.have.length(0); + }); + + it('rejects native value riding along token-shaped calldata when the caller asked for the check', async function () { + // transfer(baseAddress, 0) passes the recipient check, so without the value check the + // native balance would go to whatever contract `to` names. + const txHex = await build({ + to: attackerAddress, + value: '5000', + data: encodeErc20Transfer(baseAddress, '0'), + }); + for (const strict of [false, true]) { + await verify(consolidate, txHex, { strict, verification: toBase }).should.be.rejectedWith( + /must not carry native value/ + ); + } + }); + + it('flags the same transaction on a consolidate intent (warn, throw in strict mode)', async function () { + const txHex = await build({ + to: attackerAddress, + value: '5000', + data: encodeErc20Transfer(baseAddress, '0'), + }); + (await verify(consolidate, txHex)).should.equal(true); + warnings.some((w) => /must not carry native value/.test(w.reason)).should.equal(true); + await verify(consolidate, txHex, { strict: true }).should.be.rejectedWith(/must not carry native value/); + }); + }); + + describe('unsupported and unhandled intent types', function () { + it('rejects contractCall and customTx outright on EVM (INFOSEC-671)', async function () { + const txHex = await build({ to: attackerAddress, value: '0', data: '0xdeadbeef' }); + for (const type of ['contractCall', 'customTx']) { + for (const strict of [false, true]) { + await verify({ type, recipients: [{ address: attackerAddress, amount: '0' }] }, txHex, { + strict, + }).should.be.rejectedWith(/unsupported intent type for EVM TSS/); + } + } + }); + + it('flags every intent type without a handler instead of silently passing', async function () { + const txHex = await build({ to: attackerAddress, value: '0', data: '0xdeadbeef' }); + for (const type of ['acceleration', 'bridgeFunds', 'enableToken', 'defiRedeem', 'stakeWithCallData']) { + warnings = []; + const result = await verify({ type, recipients: [] }, txHex); + result.should.equal(true, `${type} must not throw`); + warnings.some((w) => /no fail-closed verifier/.test(w.reason)).should.equal(true, `${type} must be flagged`); + } + await verify({ type: 'bridgeFunds', recipients: [] }, txHex, { strict: true }).should.be.rejectedWith( + /no fail-closed verifier/ + ); + }); + }); + + describe('pending-approval path (stored intent via resolveEffectiveTxParams)', function () { + const verifyPendingApproval = async ( + intent: Record, + txHex: string, + strict = false + ): Promise => { + const txRequest = { txRequestId: 'pa', walletId: 'w', intent } as unknown as TxRequest; + const txParams = resolveEffectiveTxParams(txRequest, undefined, chain); + return verify(txParams as unknown as Record, txHex, { + strict, + intent: txRequest.intent as PopulatedIntent, + }); + }; + + it('keeps the existing native checks throwing on this path (redirect and inflated value)', async function () { + const intent = { + intentType: 'payment', + recipients: [{ address: { address: recipientAddress }, amount: { value: '1000', symbol: 'eth' } }], + }; + const redirectHex = await build({ to: attackerAddress, value: '1000' }); + await verifyPendingApproval(intent, redirectHex).should.be.rejectedWith(/destination address does not match/); + + const inflatedHex = await build({ to: recipientAddress, value: '9999' }); + await verifyPendingApproval(intent, inflatedHex).should.be.rejectedWith(/amount in txPrebuild does not match/); + }); + + it('flags swapped calldata on this path (warn, throw in strict mode)', async function () { + const data = '0xdeadbeef'; + const intent = { + intentType: 'payment', + recipients: [{ address: { address: tokenContractAddress }, amount: { value: '0', symbol: 'eth' }, data }], + }; + const txHex = await build({ to: tokenContractAddress, value: '0', data: '0xdeadbe00' }); + (await verifyPendingApproval(intent, txHex)).should.equal(true); + warnings.some((w) => /calldata does not match/.test(w.reason)).should.equal(true); + await verifyPendingApproval(intent, txHex, true).should.be.rejectedWith(/calldata does not match/); + }); + + it('verifies a matching payment intent on this path with no warnings', async function () { + const data = '0xdeadbeef'; + const intent = { + intentType: 'payment', + recipients: [{ address: { address: tokenContractAddress }, amount: { value: '0', symbol: 'eth' }, data }], + }; + const txHex = await build({ to: tokenContractAddress, value: '0', data }); + (await verifyPendingApproval(intent, txHex)).should.equal(true); + warnings.should.have.length(0); + }); + + it('verifies stored defi-* intents without InvalidTransactionError and without a silent skip', async function () { + const defiCases: Array<{ intentType: string; data: string; tampered: string; amountField: string }> = [ + { + intentType: 'defi-approve', + data: encodeApprove(attackerAddress, '123'), + tampered: encodeApprove(attackerAddress, '999'), + amountField: 'amount', + }, + { + intentType: 'defi-deposit', + data: encodeWithAbi('deposit', ['uint256', 'address'], ['123', baseAddress]), + tampered: encodeWithAbi('deposit', ['uint256', 'address'], ['999', baseAddress]), + amountField: 'amount', + }, + { + intentType: 'defi-withdraw', + data: encodeWithAbi('redeem', ['uint256', 'address', 'address'], ['123', baseAddress, baseAddress]), + tampered: encodeWithAbi('redeem', ['uint256', 'address', 'address'], ['999', baseAddress, baseAddress]), + amountField: 'shareTokenAmount', + }, + ]; + for (const { intentType, data, tampered, amountField } of defiCases) { + warnings = []; + const intent: Record = { intentType, vaultId: 'vault', [amountField]: '123' }; + const txHex = await build({ to: tokenContractAddress, value: '0', data }); + const result = await verifyPendingApproval(intent, txHex); + result.should.equal(true, `${intentType} must verify`); + + // Not a silent skip: tampered amounts are rejected by the DeFi calldata checks. + const tamperedHex = await build({ to: tokenContractAddress, value: '0', data: tampered }); + await verifyPendingApproval(intent, tamperedHex).should.be.rejectedWith( + /does not match the requested amount|do not match the requested amount/ + ); + } + }); + + it('keeps rejecting stored defi-redeem intents, which have no handler, as master does', async function () { + const intent = { intentType: 'defi-redeem', vaultId: 'v', amount: '1' }; + const txHex = await build({ to: tokenContractAddress, value: '0', data: '0xdeadbeef' }); + for (const strict of [false, true]) { + await verifyPendingApproval(intent, txHex, strict).should.be.rejectedWith(/Recipient details are required/); + } + warnings.should.have.length(0); + }); + }); + + describe('legacy (non-EIP-1559) chains run the parent verifier', function () { + it('rejects contractCall outright instead of returning true', async function () { + const txHex = await build({ to: attackerAddress, value: '0', data: '0xdeadbeef' }); + await verify( + { type: 'contractCall', recipients: [{ address: attackerAddress, amount: '0' }] }, + txHex + ).should.be.rejectedWith(/unsupported intent type for EVM TSS/); + }); + + it('flags a payment with unrecognized calldata instead of returning true', async function () { + const txHex = await build({ + to: tokenContractAddress, + value: '0', + data: encodeApprove(attackerAddress, MAX_UINT256), + }); + const txParams = { type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }; + (await verify(txParams, txHex)).should.equal(true); + warnings.some((w) => /not a recognized payment shape/.test(w.reason)).should.equal(true); + }); + }); + + describe('warning infrastructure', function () { + it('reports decoded vs expected to / value / data through onVerificationWarning', async function () { + const data = '0xdeadbeef'; + const txHex = await build({ to: attackerAddress, value: '7', data: '0xdeadbe00' }); + await verify({ type: 'transfer', recipients: [{ address: tokenContractAddress, amount: '0', data }] }, txHex); + warnings.length.should.be.greaterThanOrEqual(3); + const destinationWarning = warnings.find((w) => /to does not match/.test(w.reason)); + destinationWarning?.expected.to?.toLowerCase().should.equal(tokenContractAddress.toLowerCase()); + destinationWarning?.decoded.to?.toLowerCase().should.equal(attackerAddress.toLowerCase()); + const valueWarning = warnings.find((w) => /value does not match/.test(w.reason)); + valueWarning?.expected.value?.should.equal('0'); + valueWarning?.decoded.value?.should.equal('7'); + const dataWarning = warnings.find((w) => /calldata does not match/.test(w.reason)); + dataWarning?.expected.data?.should.equal('0xdeadbeef'); + dataWarning?.decoded.data?.should.equal('0xdeadbe00'); + }); + + it('does not fire the callback when the transaction matches the intent', async function () { + const txHex = await build({ to: recipientAddress, value: '1000' }); + await verify({ type: 'transfer', recipients: [{ address: recipientAddress, amount: '1000' }] }, txHex); + warnings.should.have.length(0); + }); + }); + }); +}); diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index cb4ec7f61c7..a3423db3f1d 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -315,6 +315,35 @@ export interface VerificationOptions { consolidationToBaseAddress?: boolean; } +/** + * Decoded vs expected transaction fields reported by fail-closed TSS verification. + * Undefined fields were not applicable to the violated rule. + */ +export interface TssVerificationMismatch { + to?: string; + value?: string; + data?: string; +} + +/** + * A non-fatal TSS verification finding: the transaction bytes about to be signed do not + * match the user intent. Reported through `console.warn` and an optional + * `onVerificationWarning` callback; becomes a hard failure when the client opts into + * `strictTssVerification`. + */ +export interface TssVerificationWarning { + /** Canonical verifier intent type the handler ran under (e.g. 'transfer'). */ + intentType: string; + /** Human-readable description of the violated rule. */ + reason: string; + /** Values derived from the user intent (txParams or the stored tx request intent). */ + expected: TssVerificationMismatch; + /** Values decoded from the transaction bytes that are about to be signed. */ + decoded: TssVerificationMismatch; +} + +export type TssVerificationWarningHandler = (warning: TssVerificationWarning) => void; + export interface VerifyTransactionOptions { txPrebuild: TransactionPrebuild; txParams: TransactionParams; @@ -322,6 +351,26 @@ export interface VerifyTransactionOptions { verification?: VerificationOptions; reqId?: IRequestTracer; walletType?: 'onchain' | 'tss'; + /** + * The persisted TxRequest intent (`PopulatedIntent`), 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/bitgoBase.ts b/modules/sdk-core/src/bitgo/bitgoBase.ts index 698e4846fd4..5cca2f1fb0a 100644 --- a/modules/sdk-core/src/bitgo/bitgoBase.ts +++ b/modules/sdk-core/src/bitgo/bitgoBase.ts @@ -8,7 +8,7 @@ import { IEncryptionSession, IRequestTracer, } from '../api'; -import { IBaseCoin } from './baseCoin'; +import { IBaseCoin, TssVerificationWarningHandler } from './baseCoin'; import { CoinConstructor } from './coinFactory'; import { EnvironmentName } from './environments'; import { EcdhDerivedKeypair, GetSigningKeyApi } from './keychain'; @@ -61,4 +61,14 @@ export interface BitGoBase { setRequestTracer(reqTracer: IRequestTracer): void; url(path: string, version?: number): string; register(name: string, coin: CoinConstructor): void; + /** + * Client-side opt-in: fail-closed TSS verification warnings become hard failures. + * Set only from local `BitGoAPI` options — never read from server data. + */ + readonly strictTssVerification?: boolean; + /** + * Optional sink for fail-closed TSS verification warnings (decoded vs expected + * transaction fields). Set only from local `BitGoAPI` options. + */ + readonly onVerificationWarning?: TssVerificationWarningHandler; } 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..b919b44ecbd 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, @@ -832,16 +833,20 @@ export class EcdsaUtils extends BaseEcdsaUtils { if (shouldVerifyWithSerializedTxHex(this.baseCoin)) { 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, }); } 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..1064182bd6f 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, @@ -991,9 +992,11 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { if (shouldVerifyWithSerializedTxHex(this.baseCoin) || shouldUsePreHashedSignable(this.baseCoin, unsignedTx)) { 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)) { this.baseCoin.assertSignableConsistency(unsignedTx.serializedTxHex, unsignedTx.signableHex); @@ -1001,9 +1004,11 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils { } 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..9cdc6edf0ce 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -29,10 +29,20 @@ export const NO_RECIPIENT_TX_TYPES = new Set([ 'disableToken', 'disabletoken', 'customTx', - // DeFi vault operations — recipients/calldata built server-side from defiParams + // DeFi vault operations — recipients/calldata built server-side from defiParams. + // Both spellings registered on purpose (see the wrapNative note below): the stored + // WP intentType is kebab-case while buildParams.type is camelCase. These types are + // NOT skipped by verifyTssTransaction — the DeFi handlers decode and compare the + // calldata against defiParams; this entry only waives the missing-recipients guard. 'defiApprove', + 'defi-approve', 'defiDeposit', + 'defi-deposit', 'defiWithdraw', + 'defi-withdraw', + // defi-redeem is deliberately absent: it has no EVM verifier handler, so a stored + // defi-redeem intent keeps failing the missing-recipients guard instead of reaching + // signing with only a warning. // Native wrap/unwrap (WETH9 deposit()/withdraw()) — calldata and the WETH9 // address are resolved server-side from the vault binding, so no recipients. // Registered in BOTH spellings on purpose: this set is matched against 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..b262f970e06 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 @@ -19,6 +19,7 @@ import { EcdsaMPCv2Utils, EcdsaMPCv2KeyGenCallbacks, IBaseCoin, + IRequestTracer, IWallet, RequestTracer, SignatureShareRecord, @@ -26,6 +27,7 @@ import { TxRequest, } from '../../../../../../src'; import { bitgoGpgKey } from './gpgKeys'; +import { readKey } from 'openpgp'; describe('ECDSA MPC v2', async () => { let userShare: Buffer; @@ -636,6 +638,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 +710,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 +785,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 = { @@ -827,7 +832,94 @@ describe('ECDSA MPC v2', async () => { const verifyCallArgs = verifyTransactionSpy.firstCall.args[0]; assert.strictEqual(verifyCallArgs.txParams.type, intentType); assert.strictEqual(verifyCallArgs.txParams.recipients, undefined); + // The stored intent must reach the verifier so fail-closed EVM checks can compare + // decoded to / value / data against it on the pending-approval path. + assert.strictEqual( + verifyCallArgs.intent, + (txRequest as any).intent, + 'stored intent must reach verifyTransaction' + ); + assert.strictEqual( + verifyCallArgs.callerTxParams, + undefined, + 'pending approval supplies no caller txParams; the stored intent is the only source' + ); + }); + }); + + it('signRequestBase should pass the caller txParams and the stored intent to verifyTransaction', async function () { + const serializedTxHex = 'f86c808504a817c80082520894' + '00'.repeat(20) + '80808080'; + const callerTxParams = { + type: 'transfer', + recipients: [{ address: '0x' + '11'.repeat(20), amount: '1000' }], + }; + const intent = { + intentType: 'payment', + recipients: [{ address: { address: '0x' + '11'.repeat(20) }, amount: { value: '1000', symbol: 'hteth' } }], + }; + + const mockBgWithPost = {} as BitGoBase; + mockBgWithPost.getEnv = sinon.stub().returns('test'); + mockBgWithPost.setRequestTracer = sinon.stub(); + mockBgWithPost.encrypt = sinon.stub().resolves('encrypted'); + mockBgWithPost.decrypt = sinon.stub().resolves('decrypted'); + mockBgWithPost.post = sinon.stub().returns({ + send: sinon.stub().returnsThis(), + set: sinon.stub().returnsThis(), + result: sinon.stub().rejects(new Error('mock: HTTP not available')), }); + + const verifyTransactionSpy = sinon.stub().resolves(true); + const mockCoinForDirectSend = { + getHashFunction: sinon.stub().callsFake(() => createKeccakHash('keccak256') as Hash), + verifyTransaction: verifyTransactionSpy, + getMPCAlgorithm: sinon.stub().returns('ecdsa'), + getConfig: sinon.stub().returns({ family: 'hteth' }), + getChain: sinon.stub().returns('hteth'), + } as unknown as IBaseCoin; + + const mockWallet = { + id: sinon.stub().returns(walletID), + multisigType: sinon.stub().returns('tss'), + multisigTypeVersion: sinon.stub().returns('MPCv2'), + }; + + const directUtils = new EcdsaMPCv2Utils(mockBgWithPost, mockCoinForDirectSend, mockWallet as unknown as IWallet); + const bitgoPubKey = await readKey({ armoredKey: bitgoGpgKey.public }); + sinon.stub(directUtils, 'pickBitgoPubGpgKeyForSigning').resolves(bitgoPubKey); + + const txRequest = { + txRequestId: 'direct-send-test', + apiVersion: 'full', + walletId: walletID, + intent, + transactions: [ + { + unsignedTx: { derivationPath: 'm/0', signableHex: serializedTxHex, serializedTxHex }, + signatureShares: [], + }, + ], + } as unknown as TxRequest; + + try { + await directUtils.signTxRequest({ + txRequest, + txParams: callerTxParams, + prv: userShare.toString('base64'), + reqId: { inc: sinon.stub(), toString: sinon.stub().returns('test-req') } as unknown as IRequestTracer, + }); + } catch (e) {} + + assert.strictEqual(verifyTransactionSpy.callCount, 1, 'verifyTransaction must be reached exactly once'); + const verifyCallArgs = verifyTransactionSpy.firstCall.args[0]; + assert.strictEqual(verifyCallArgs.intent, intent, 'stored intent must reach verifyTransaction'); + assert.deepStrictEqual( + verifyCallArgs.callerTxParams, + callerTxParams, + 'the untouched caller input must reach the verifier' + ); + assert.strictEqual(verifyCallArgs.txParams.type, 'transfer'); + assert.deepStrictEqual(verifyCallArgs.txParams.recipients, callerTxParams.recipients); }); it('should still apply keccak256 for regular FLR EVM transactions', async () => { diff --git a/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaVerifyTransactionWiring.ts b/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaVerifyTransactionWiring.ts new file mode 100644 index 00000000000..2156e3e808c --- /dev/null +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaVerifyTransactionWiring.ts @@ -0,0 +1,132 @@ +import * as assert from 'assert'; +import * as sinon from 'sinon'; +import { + BitGoBase, + EcdsaUtils, + IBaseCoin, + IRequestTracer, + IWallet, + TssSignTxRequestParamsWithPrv, + TxRequest, +} from '../../../../../../src'; + +/** + * CECHO-2460: the EVM TSS verifier must receive both the caller's own txParams and the stored + * tx request intent. `txParams` is the post-`resolveEffectiveTxParams` object (the stored intent + * is back-filled into it), so it cannot tell the verifier whether the caller declared anything; + * `callerTxParams` is the untouched caller input that decides which source is authoritative. + * + * This pins the ECDSA (MPCv1) wiring. EcdsaMPCv2Utils has the equivalent test in ecdsaMPCv2.ts. + */ +describe('EcdsaUtils signing verifyTransaction wiring', function () { + const walletID = '62fe536a6b4cf70007acb48c0e7bb0b0'; + const derivationPath = 'm/0'; + const signableHex = 'f86c808504a817c80082520894' + '00'.repeat(20) + '80808080'; + + const makeTxRequest = (intent: Record): TxRequest => + ({ + txRequestId: 'ecdsa-wiring', + apiVersion: 'full', + walletId: walletID, + intent, + transactions: [ + { + unsignedTx: { derivationPath, signableHex, serializedTxHex: signableHex }, + signatureShares: [], + }, + ], + } as unknown as TxRequest); + + // 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: {} }); + + let verifyTransactionStub: sinon.SinonStub; + + const signWith = async (txParams: TssSignTxRequestParamsWithPrv['txParams'], intent: Record) => { + const mockBitgo = { + getEnv: sinon.stub().returns('test'), + setRequestTracer: sinon.stub(), + } as unknown as BitGoBase; + const mockCoin = { + verifyTransaction: verifyTransactionStub, + getConfig: sinon.stub().returns({}), + getChain: sinon.stub().returns('hteth'), + } as unknown as IBaseCoin; + const mockWallet = { + id: sinon.stub().returns(walletID), + multisigType: sinon.stub().returns('tss'), + coinSpecific: sinon.stub().returns(undefined), + }; + + const utils = new EcdsaUtils(mockBitgo, mockCoin, mockWallet as unknown as IWallet); + // Stop right after the verifier: the TSS networking that follows is covered elsewhere. + sinon.stub(utils, 'getEcdsaSigningChallenges').rejects(new Error('stop-after-verify')); + + try { + await utils.signTxRequest({ + txRequest: makeTxRequest(intent), + txParams, + prv, + reqId: { inc: sinon.stub(), toString: sinon.stub().returns('test-req') } as unknown as IRequestTracer, + }); + } catch (e) { + assert.match((e as Error).message, /stop-after-verify/); + } + + assert.strictEqual(verifyTransactionStub.callCount, 1, 'verifyTransaction must be reached exactly once'); + return verifyTransactionStub.firstCall.args[0] as { + txParams: { type?: string; recipients?: unknown[] }; + intent?: Record; + callerTxParams?: unknown; + }; + }; + + beforeEach(function () { + verifyTransactionStub = sinon.stub().resolves(true); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('passes the caller txParams and the stored intent to the verifier on a direct send', async function () { + const callerTxParams = { + type: 'transfer', + recipients: [{ address: '0x' + '11'.repeat(20), amount: '1000' }], + }; + const intent = { + intentType: 'payment', + recipients: [{ address: { address: '0x' + '11'.repeat(20) }, amount: { value: '1000', symbol: 'hteth' } }], + }; + + const verifyArgs = await signWith(callerTxParams, intent); + + assert.strictEqual(verifyArgs.intent, intent, 'stored intent must reach verifyTransaction'); + assert.deepStrictEqual( + verifyArgs.callerTxParams, + callerTxParams, + 'the untouched caller input must reach the verifier' + ); + assert.strictEqual(verifyArgs.txParams.type, 'transfer'); + assert.deepStrictEqual(verifyArgs.txParams.recipients, callerTxParams.recipients); + }); + + it('passes callerTxParams as undefined on the pending-approval path', async function () { + const intent = { + intentType: 'payment', + recipients: [{ address: { address: '0x' + '22'.repeat(20) }, amount: { value: '1000', symbol: 'hteth' } }], + }; + + const verifyArgs = await signWith(undefined, intent); + + assert.strictEqual(verifyArgs.intent, intent, 'stored intent must reach verifyTransaction'); + assert.strictEqual( + verifyArgs.callerTxParams, + undefined, + 'the pending-approval path supplies no caller params; the stored intent speaks' + ); + assert.strictEqual(verifyArgs.txParams.type, 'payment'); + assert.strictEqual(verifyArgs.txParams.recipients?.length, 1, 'stored recipients are back-filled'); + }); +}); 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..fb615f12b6c 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/recipientUtils.ts @@ -29,8 +29,11 @@ describe('recipientUtils', function () { 'disabletoken', 'customTx', 'defiApprove', + 'defi-approve', 'defiDeposit', + 'defi-deposit', 'defiWithdraw', + 'defi-withdraw', // Native wrap/unwrap — registered in both spellings on purpose 'wrapNative', 'wrap-native', @@ -82,10 +85,31 @@ describe('recipientUtils', function () { }); it('does not contain value-transfer types', function () { - ['payment', 'fanout', 'vote', 'defi-deposit', 'defi-redeem'].forEach((t) => { + ['payment', 'fanout', 'vote'].forEach((t) => { assert.ok(!NO_RECIPIENT_TX_TYPES.has(t), `${t} must NOT be in NO_RECIPIENT_TX_TYPES`); }); }); + + it('accepts kebab-case stored DeFi intent types without recipients', function () { + // Stored defi-* intents have no recipients; they must reach verifyTssTransaction's DeFi + // calldata checks (no InvalidTransactionError, no silent skip) instead of being rejected + // by the missing-recipients guard. + for (const intentType of ['defi-approve', 'defi-deposit', 'defi-withdraw']) { + const txRequest = makeTxRequest({ intent: { intentType } as PopulatedIntent }); + assert.doesNotThrow( + () => resolveEffectiveTxParams(txRequest, undefined), + `${intentType} should not throw without recipients` + ); + } + }); + + it('keeps rejecting stored defi-redeem intents, which have no EVM verifier handler', function () { + for (const intentType of ['defi-redeem', 'defiRedeem']) { + assert.ok(!NO_RECIPIENT_TX_TYPES.has(intentType), `${intentType} must NOT be in NO_RECIPIENT_TX_TYPES`); + const txRequest = makeTxRequest({ intent: { intentType } as PopulatedIntent }); + assert.throws(() => resolveEffectiveTxParams(txRequest, undefined), InvalidTransactionError); + } + }); }); describe('resolveEffectiveTxParams', function () {