diff --git a/modules/sdk-api/src/bitgoAPI.ts b/modules/sdk-api/src/bitgoAPI.ts index 504868e629..0b07559ea8 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'; @@ -271,10 +272,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 29d7a1af77..cff9eb2da1 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 { ECPairInterface } from '@bitgo/utxo-lib'; import { type Agent } from 'http'; @@ -70,6 +76,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-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index cb4ec7f61c..ccaa658219 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,14 @@ 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; } export interface SupplementGenerateWalletOptions { diff --git a/modules/sdk-core/src/bitgo/bitgoBase.ts b/modules/sdk-core/src/bitgo/bitgoBase.ts index 698e4846fd..5cca2f1fb0 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 0473a7ee39..d0649c6f23 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 fece8b88d0..a308b5db8b 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,18 @@ 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, }); } 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, }); } 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 b942656b94..d558fdf3dc 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,10 @@ 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, }); if (shouldVerifyWithSerializedTxHex(this.baseCoin) && isCoinWithSignableConsistency(this.baseCoin)) { this.baseCoin.assertSignableConsistency(unsignedTx.serializedTxHex, unsignedTx.signableHex); @@ -1001,9 +1003,10 @@ 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, }); } 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 f33d44b9b0..d2054a7878 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/recipientUtils.ts @@ -29,10 +29,22 @@ 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 has no EVM verifier handler and is flagged as a violation there; + // listed here so the pending-approval path reaches the verifier instead of + // failing earlier on the missing-recipients guard. + 'defi-redeem', + 'defiRedeem', // 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 a72732aeeb..7942a0293e 100644 --- a/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/modules/sdk-core/test/unit/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts @@ -636,6 +636,7 @@ describe('ECDSA MPC v2', async () => { verifyTransaction: sinon.stub().resolves(true), getMPCAlgorithm: sinon.stub().returns('ecdsa'), getConfig: sinon.stub().returns({ family: 'flrp' }), + getChain: sinon.stub().returns('flrp'), isSignablePreHashed: (unsignedTx: { serializedTxHex?: string }) => unsignedTx.serializedTxHex?.startsWith('0000') ?? false, } as unknown as IBaseCoin; @@ -707,6 +708,7 @@ describe('ECDSA MPC v2', async () => { verifyTransaction: sinon.stub().resolves(true), getMPCAlgorithm: sinon.stub().returns('ecdsa'), getConfig: sinon.stub().returns({ family: 'flr' }), + getChain: sinon.stub().returns('flr'), } as unknown as IBaseCoin; const mockWallet = { @@ -781,6 +783,7 @@ describe('ECDSA MPC v2', async () => { verifyTransaction: verifyTransactionSpy, getMPCAlgorithm: sinon.stub().returns('ecdsa'), getConfig: sinon.stub().returns({ family: 'hteth' }), + getChain: sinon.stub().returns('hteth'), } as unknown as IBaseCoin; const mockWallet = { @@ -827,6 +830,13 @@ 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' + ); }); }); 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 bb6ffbf568..c015989e53 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,15 @@ describe('recipientUtils', function () { 'disabletoken', 'customTx', 'defiApprove', + 'defi-approve', 'defiDeposit', + 'defi-deposit', 'defiWithdraw', + 'defi-withdraw', + // defi-redeem has no EVM verifier handler and is flagged there; listed so the + // pending-approval path reaches the verifier instead of failing the guard first + 'defiRedeem', + 'defi-redeem', // Native wrap/unwrap — registered in both spellings on purpose 'wrapNative', 'wrap-native', @@ -82,10 +89,23 @@ 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', 'defi-redeem']) { + const txRequest = makeTxRequest({ intent: { intentType } as PopulatedIntent }); + assert.doesNotThrow( + () => resolveEffectiveTxParams(txRequest, undefined), + `${intentType} should not throw without recipients` + ); + } + }); }); describe('resolveEffectiveTxParams', function () {