fix(sdk-coin-bsc,sdk-coin-xdc): harden TSS verify via serializedTxHex

modules/sdk-coin-bsc/src/bsc.ts

@@ -1,18 +1,6 @@
-import {
-  BaseCoin,
-  BitGoBase,
-  common,
-  MPCAlgorithm,
-  MultisigType,
-  multisigTypes,
-  NO_RECIPIENT_TX_TYPES,
-} from '@bitgo/sdk-core';
+import { BaseCoin, BitGoBase, common, MPCAlgorithm, MultisigType, multisigTypes } from '@bitgo/sdk-core';
 import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics';
-import {
-  AbstractEthLikeNewCoins,
-  recoveryBlockchainExplorerQuery,
-  VerifyEthTransactionOptions,
-} from '@bitgo/abstract-eth';
+import { AbstractEthLikeNewCoins, recoveryBlockchainExplorerQuery } from '@bitgo/abstract-eth';
 import { TransactionBuilder } from './lib';
 
 export class Bsc extends AbstractEthLikeNewCoins {
@@ -62,36 +50,4 @@ export class Bsc extends AbstractEthLikeNewCoins {
     const explorerUrl = common.Environments[this.bitgo.getEnv()].bscscanBaseUrl;
     return await recoveryBlockchainExplorerQuery(query, explorerUrl as string, apiToken);
   }
-
-  /**
-   * Verify if a tss transaction is valid
-   *
-   * @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
-   * @returns {boolean}
-   */
-  async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
-    const { txParams, txPrebuild, wallet } = params;
-    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`);
-    }
-
-    return true;
-  }
 }

modules/sdk-coin-bsc/src/bscToken.ts

@@ -3,8 +3,8 @@
  */
 
 import { EthLikeTokenConfig, coins } from '@bitgo/statics';
-import { BitGoBase, CoinConstructor, NamedCoinConstructor, MPCAlgorithm, NO_RECIPIENT_TX_TYPES } from '@bitgo/sdk-core';
-import { CoinNames, EthLikeToken, VerifyEthTransactionOptions } from '@bitgo/abstract-eth';
+import { BitGoBase, CoinConstructor, MPCAlgorithm, NamedCoinConstructor } from '@bitgo/sdk-core';
+import { CoinNames, EthLikeToken } from '@bitgo/abstract-eth';
 import { TransactionBuilder } from './lib';
 
 export { EthLikeTokenConfig };
@@ -43,35 +43,4 @@ export class BscToken extends EthLikeToken {
   getFullName(): string {
     return 'Bsc Token';
   }
-  /**
-   * Verify if a tss transaction is valid
-   *
-   * @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
-   * @returns {boolean}
-   */
-  async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
-    const { txParams, txPrebuild, wallet } = params;
-    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`);
-    }
-
-    return true;
-  }
 }

modules/sdk-coin-bsc/test/unit/bsc.ts

@@ -2,8 +2,18 @@ import 'should';
 
 import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test';
 import { BitGoAPI } from '@bitgo/sdk-api';
+import { TransactionType, Wallet } from '@bitgo/sdk-core';
 
 import { Bsc, Tbsc } from '../../src/index';
+import { TransactionBuilder } from '../../src/lib';
+import { getBuilder } from './getBuilder';
+
+/** Encode ERC-20 transfer(address,uint256) calldata without ethereumjs-abi. */
+function encodeErc20Transfer(to: string, amount: string): string {
+  const address = to.toLowerCase().replace(/^0x/, '').padStart(64, '0');
+  const value = BigInt(amount).toString(16).padStart(64, '0');
+  return `0xa9059cbb${address}${value}`;
+}
 
 const bitgo: TestBitGoAPI = TestBitGo.decorate(BitGoAPI, { env: 'test' });
 
@@ -39,4 +49,178 @@ describe('Native BNB', function () {
       tbsc.allowsAccountConsolidations().should.equal(true);
     });
   });
+
+  describe('verifyTssTransaction', function () {
+    const recipientAddress = '0x174cfd823af8ce27ed0afee3fcf3c3ba259116be';
+    const wrongAddress = '0x7e85bdc27c050e3905ebf4b8e634d9ad6edd0de6';
+    const tokenContractAddress = '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48';
+    const transferAmount = '1000000000000000000';
+
+    it('should accept a native BNB transfer where txHex matches declared recipient', async function () {
+      const coin = bitgo.coin('tbsc') as Tbsc;
+
+      const txBuilder = getBuilder('tbsc') as TransactionBuilder;
+      txBuilder.type(TransactionType.SingleSigSend);
+      txBuilder.fee({ fee: '10', gasLimit: '21000' });
+      txBuilder.counter(1);
+      txBuilder.contract(recipientAddress);
+      txBuilder.value(transferAmount);
+      const tx = await txBuilder.build();
+      const txHex = tx.toBroadcastFormat();
+
+      const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
+
+      const result = await coin.verifyTssTransaction({
+        txParams: {
+          type: 'transfer',
+          recipients: [{ address: recipientAddress, amount: transferAmount }],
+        } as any,
+        txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
+        wallet,
+      });
+      result.should.equal(true);
+    });
+
+    it('should reject a native BNB transfer when txHex recipient does not match declared recipient', async function () {
+      const coin = bitgo.coin('tbsc') as Tbsc;
+
+      const txBuilder = getBuilder('tbsc') as TransactionBuilder;
+      txBuilder.type(TransactionType.SingleSigSend);
+      txBuilder.fee({ fee: '10', gasLimit: '21000' });
+      txBuilder.counter(1);
+      txBuilder.contract(wrongAddress);
+      txBuilder.value(transferAmount);
+      const tx = await txBuilder.build();
+      const txHex = tx.toBroadcastFormat();
+
+      const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
+
+      await coin
+        .verifyTssTransaction({
+          txParams: {
+            type: 'transfer',
+            recipients: [{ address: recipientAddress, amount: transferAmount }],
+          } as any,
+          txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
+          wallet,
+        })
+        .should.be.rejectedWith('destination address does not match with the recipient address');
+    });
+
+    it('should accept a BEP-20 token transfer where calldata matches declared recipient', async function () {
+      const coin = bitgo.coin('tbsc') as Tbsc;
+
+      const erc20TransferData = encodeErc20Transfer(recipientAddress, '10000000');
+
+      const txBuilder = getBuilder('tbsc') as TransactionBuilder;
+      txBuilder.type(TransactionType.ContractCall);
+      txBuilder.fee({ fee: '10', gasLimit: '60000' });
+      txBuilder.counter(1);
+      txBuilder.contract(tokenContractAddress);
+      txBuilder.data(erc20TransferData);
+      const tx = await txBuilder.build();
+      const txHex = tx.toBroadcastFormat();
+
+      const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
+
+      const result = await coin.verifyTssTransaction({
+        txParams: {
+          type: 'transfer',
+          recipients: [{ address: recipientAddress, amount: '10000000' }],
+        } as any,
+        txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
+        wallet,
+      });
+      result.should.equal(true);
+    });
+
+    it('should reject a BEP-20 token transfer when calldata recipient does not match declared recipient', async function () {
+      const coin = bitgo.coin('tbsc') as Tbsc;
+
+      const erc20TransferData = encodeErc20Transfer(wrongAddress, '10000000');
+
+      const txBuilder = getBuilder('tbsc') as TransactionBuilder;
+      txBuilder.type(TransactionType.ContractCall);
+      txBuilder.fee({ fee: '10', gasLimit: '60000' });
+      txBuilder.counter(1);
+      txBuilder.contract(tokenContractAddress);
+      txBuilder.data(erc20TransferData);
+      const tx = await txBuilder.build();
+      const txHex = tx.toBroadcastFormat();
+
+      const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
+
+      await coin
+        .verifyTssTransaction({
+          txParams: {
+            type: 'transfer',
+            recipients: [{ address: recipientAddress, amount: '10000000' }],
+          } as any,
+          txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
+          wallet,
+        })
+        .should.be.rejectedWith('destination address does not match with the recipient address');
+    });
+
+    it('should accept a BEP-20 token transfer using WalletConnect recipients[0].data flow', async function () {
+      const coin = bitgo.coin('tbsc') as Tbsc;
+
+      // txHex sends to recipientAddress; recipients[0].data encodes the same intent
+      const erc20TransferData = encodeErc20Transfer(recipientAddress, '10000000');
+
+      const txBuilder = getBuilder('tbsc') as TransactionBuilder;
+      txBuilder.type(TransactionType.ContractCall);
+      txBuilder.fee({ fee: '10', gasLimit: '60000' });
+      txBuilder.counter(1);
+      txBuilder.contract(tokenContractAddress);
+      txBuilder.data(erc20TransferData);
+      const tx = await txBuilder.build();
+      const txHex = tx.toBroadcastFormat();
+
+      const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
+
+      const result = await coin.verifyTssTransaction({
+        txParams: {
+          type: 'transfer',
+          // WalletConnect passes the intended calldata in recipients[0].data
+          recipients: [{ address: tokenContractAddress, amount: '0', data: erc20TransferData }],
+        } as any,
+        txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
+        wallet,
+      });
+      result.should.equal(true);
+    });
+
+    it('should reject a BEP-20 token transfer using WalletConnect flow when calldata recipient is tampered', async function () {
+      const coin = bitgo.coin('tbsc') as Tbsc;
+
+      // txHex sends to wrongAddress (tampered)
+      const tamperedData = encodeErc20Transfer(wrongAddress, '10000000');
+
+      const txBuilder = getBuilder('tbsc') as TransactionBuilder;
+      txBuilder.type(TransactionType.ContractCall);
+      txBuilder.fee({ fee: '10', gasLimit: '60000' });
+      txBuilder.counter(1);
+      txBuilder.contract(tokenContractAddress);
+      txBuilder.data(tamperedData);
+      const tx = await txBuilder.build();
+      const txHex = tx.toBroadcastFormat();
+
+      // recipients[0].data declares the correct recipient (recipientAddress)
+      const correctData = encodeErc20Transfer(recipientAddress, '10000000');
+
+      const wallet = new Wallet(bitgo, coin, { coinSpecific: { baseAddress: recipientAddress } });
+
+      await coin
+        .verifyTssTransaction({
+          txParams: {
+            type: 'transfer',
+            recipients: [{ address: tokenContractAddress, amount: '0', data: correctData }],
+          } as any,
+          txPrebuild: { txHex, coin: 'tbsc', walletId: 'fakeWalletId' } as any,
+          wallet,
+        })
+        .should.be.rejectedWith('destination address does not match with the recipient address');
+    });
+  });
 });

modules/sdk-coin-xdc/src/xdc.ts

@@ -1,20 +1,11 @@
-import {
-  BaseCoin,
-  BitGoBase,
-  common,
-  MPCAlgorithm,
-  MultisigType,
-  multisigTypes,
-  NO_RECIPIENT_TX_TYPES,
-} from '@bitgo/sdk-core';
+import { BaseCoin, BitGoBase, common, MPCAlgorithm, MultisigType, multisigTypes } from '@bitgo/sdk-core';
 import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics';
 import {
   AbstractEthLikeNewCoins,
   recoveryBlockchainExplorerQuery,
   UnsignedSweepTxMPCv2,
   RecoverOptions,
   OfflineVaultTxInfo,
-  VerifyEthTransactionOptions,
 } from '@bitgo/abstract-eth';
 import { TransactionBuilder } from './lib';
 
@@ -55,35 +46,4 @@ export class Xdc extends AbstractEthLikeNewCoins {
     const explorerUrl = common.Environments[this.bitgo.getEnv()].xdcExplorerBaseUrl;
     return await recoveryBlockchainExplorerQuery(query, explorerUrl as string, apiToken);
   }
-  /**
-   * Verify if a tss transaction is valid
-   *
-   * @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
-   * @returns {boolean}
-   */
-  async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
-    const { txParams, txPrebuild, wallet } = params;
-    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`);
-    }
-
-    return true;
-  }
 }