Merge pull request #7020 from BitGo/WP-5945/sdk-unspent-management-validation

feat(sdk-core): add transaction validation for (consolidate) unspent management

Author: danielpeng1

modules/bitgo/test/v2/unit/unspents.ts

@@ -60,6 +60,7 @@ describe('Verify string type is used for value of unspent', function () {
           );
 
         sinon.stub(wallet, 'signTransaction').resolves({});
+        sinon.stub(wallet.baseCoin, 'verifyTransaction').resolves();
 
         const sendScope = nock(bgUrl)
           .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`, { type: manageUnspentType })

modules/bitgo/test/v2/unit/wallet.ts

@@ -1589,6 +1589,7 @@ describe('V2 Wallet:', function () {
           'unsigned'
         )
       );
+      psbts.forEach((psbt) => utxoLib.bitgo.addXpubsToPsbt(psbt, rootWalletKey));
       const txHexes = psbts.map((psbt) => ({ txHex: psbt.toHex() }));
 
       const nocks: nock.Scope[] = [];
@@ -1627,6 +1628,7 @@ describe('V2 Wallet:', function () {
         rootWalletKey,
         'unsigned'
       );
+      utxoLib.bitgo.addXpubsToPsbt(psbt, rootWalletKey);
 
       const nocks: nock.Scope[] = [];
       nocks.push(

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

@@ -5,9 +5,11 @@ import { btcBackupKey } from './fixtures';
 import { type TestBitGoAPI, TestBitGo } from '@bitgo/sdk-test';
 
 import { Tbtc } from '../../src';
-import { BitGoAPI } from '@bitgo/sdk-api';
+import { BitGoAPI, encrypt } from '@bitgo/sdk-api';
 import * as utxolib from '@bitgo/utxo-lib';
 
+import { Wallet } from '@bitgo/sdk-core';
+
 describe('BTC:', () => {
   let bitgo: TestBitGoAPI;
 
@@ -106,4 +108,153 @@ describe('BTC:', () => {
       );
     });
   });
+
+  describe('Unspent management spoofability - Consolidation (BUILD_SIGN_SEND)', () => {
+    let coin: Tbtc;
+    let bitgoTest: TestBitGoAPI;
+    before(() => {
+      bitgoTest = TestBitGo.decorate(BitGoAPI, { env: 'test' });
+      bitgoTest.safeRegister('tbtc', Tbtc.createInstance);
+      bitgoTest.initializeTestVars();
+      coin = bitgoTest.coin('tbtc') as Tbtc;
+    });
+
+    it('should detect hex spoofing in BUILD_SIGN_SEND', async (): Promise<void> => {
+      const keyTriple = utxolib.testutil.getKeyTriple('default');
+      const rootWalletKey = new utxolib.bitgo.RootWalletKeys(keyTriple);
+      const [user] = keyTriple;
+
+      const wallet = new Wallet(bitgoTest, coin, {
+        id: '5b34252f1bf349930e34020a',
+        coin: 'tbtc',
+        keys: ['user', 'backup', 'bitgo'],
+      });
+
+      const originalPsbt = utxolib.testutil.constructPsbt(
+        [{ scriptType: 'p2wsh' as const, value: BigInt(10000) }],
+        [{ address: 'tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3q0sl5k7', value: BigInt(9000) }],
+        coin.network,
+        rootWalletKey,
+        'unsigned' as const
+      );
+      utxolib.bitgo.addXpubsToPsbt(originalPsbt, rootWalletKey);
+      const spoofedPsbt = utxolib.testutil.constructPsbt(
+        [{ scriptType: 'p2wsh' as const, value: BigInt(10000) }],
+        [{ address: 'tb1pjgg9ty3s2ztp60v6lhgrw76f7hxydzuk9t9mjsndh3p2gf2ah7gs4850kn', value: BigInt(9000) }],
+        coin.network,
+        rootWalletKey,
+        'unsigned' as const
+      );
+      utxolib.bitgo.addXpubsToPsbt(spoofedPsbt, rootWalletKey);
+      const spoofedHex: string = spoofedPsbt.toHex();
+
+      const bgUrl: string = (bitgoTest as any)._baseUrl;
+      const nock = require('nock');
+
+      nock(bgUrl)
+        .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/consolidateUnspents`)
+        .reply(200, { txHex: spoofedHex, consolidateId: 'test' });
+
+      nock(bgUrl)
+        .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`)
+        .reply((requestBody: any) => {
+          if (requestBody?.txHex === spoofedHex) {
+            throw new Error('Spoofed transaction was sent: spoofing protection failed');
+          }
+          return [200, { txid: 'test-txid-123', status: 'signed' }];
+        });
+
+      const pubs = keyTriple.map((k) => k.neutered().toBase58());
+      const responses = [
+        { pub: pubs[0], encryptedPrv: encrypt('pass', user.toBase58()) },
+        { pub: pubs[1] },
+        { pub: pubs[2] },
+      ];
+      wallet
+        .keyIds()
+        .forEach((id, i) => nock(bgUrl).get(`/api/v2/${wallet.coin()}/key/${id}`).reply(200, responses[i]));
+
+      await assert.rejects(
+        wallet.consolidateUnspents({ walletPassphrase: 'pass' }),
+        (e: any) =>
+          typeof e?.message === 'string' &&
+          e.message.includes('prebuild attempts to spend to unintended external recipients')
+      );
+    });
+  });
+
+  describe('Unspent management spoofability - Fanout (BUILD_SIGN_SEND)', () => {
+    let coin: Tbtc;
+    let bitgoTest: TestBitGoAPI;
+    before(() => {
+      bitgoTest = TestBitGo.decorate(BitGoAPI, { env: 'test' });
+      bitgoTest.safeRegister('tbtc', Tbtc.createInstance);
+      bitgoTest.initializeTestVars();
+      coin = bitgoTest.coin('tbtc') as Tbtc;
+    });
+
+    it('should detect hex spoofing in fanout BUILD_SIGN_SEND', async (): Promise<void> => {
+      const keyTriple = utxolib.testutil.getKeyTriple('default');
+      const rootWalletKey = new utxolib.bitgo.RootWalletKeys(keyTriple);
+      const [user] = keyTriple;
+
+      const wallet = new Wallet(bitgoTest, coin, {
+        id: '5b34252f1bf349930e34020a',
+        coin: 'tbtc',
+        keys: ['user', 'backup', 'bitgo'],
+      });
+
+      const originalPsbt = utxolib.testutil.constructPsbt(
+        [{ scriptType: 'p2wsh' as const, value: BigInt(10000) }],
+        [{ address: 'tb1qrp33g0q5c5txsp9arysrx4k6zdkfs4nce4xj0gdcccefvpysxf3q0sl5k7', value: BigInt(9000) }],
+        coin.network,
+        rootWalletKey,
+        'unsigned' as const
+      );
+      utxolib.bitgo.addXpubsToPsbt(originalPsbt, rootWalletKey);
+
+      const spoofedPsbt = utxolib.testutil.constructPsbt(
+        [{ scriptType: 'p2wsh' as const, value: BigInt(10000) }],
+        [{ address: 'tb1pjgg9ty3s2ztp60v6lhgrw76f7hxydzuk9t9mjsndh3p2gf2ah7gs4850kn', value: BigInt(9000) }],
+        coin.network,
+        rootWalletKey,
+        'unsigned' as const
+      );
+      utxolib.bitgo.addXpubsToPsbt(spoofedPsbt, rootWalletKey);
+      const spoofedHex: string = spoofedPsbt.toHex();
+
+      const bgUrl: string = (bitgoTest as any)._baseUrl;
+      const nock = require('nock');
+
+      nock(bgUrl)
+        .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/fanoutUnspents`)
+        .reply(200, { txHex: spoofedHex, fanoutId: 'test' });
+
+      nock(bgUrl)
+        .post(`/api/v2/${wallet.coin()}/wallet/${wallet.id()}/tx/send`)
+        .reply((requestBody: any) => {
+          if (requestBody?.txHex === spoofedHex) {
+            throw new Error('Spoofed transaction was sent: spoofing protection failed');
+          }
+          return [200, { txid: 'test-txid-123', status: 'signed' }];
+        });
+
+      const pubs = keyTriple.map((k) => k.neutered().toBase58());
+      const responses = [
+        { pub: pubs[0], encryptedPrv: encrypt('pass', user.toBase58()) },
+        { pub: pubs[1] },
+        { pub: pubs[2] },
+      ];
+      wallet
+        .keyIds()
+        .forEach((id, i) => nock(bgUrl).get(`/api/v2/${wallet.coin()}/key/${id}`).reply(200, responses[i]));
+
+      await assert.rejects(
+        wallet.fanoutUnspents({ walletPassphrase: 'pass' }),
+        (e: any) =>
+          typeof e?.message === 'string' &&
+          e.message.includes('prebuild attempts to spend to unintended external recipients')
+      );
+    });
+  });
 });

modules/sdk-core/src/bitgo/wallet/wallet.ts

@@ -739,6 +739,27 @@ export class Wallet implements IWallet {
       .keychains()
       .getKeysForSigning({ wallet: this, reqId })) as unknown as Keychain[];
 
+    // Validate that the platform-built transaction matches user parameters
+    const txPrebuilds = Array.isArray(buildResponse) ? buildResponse : [buildResponse];
+    await Promise.all(
+      txPrebuilds.map((txPrebuild) =>
+        this.baseCoin.verifyTransaction({
+          txParams: params,
+          txPrebuild,
+          wallet: this,
+          verification: {
+            ...(params.verification ?? {}),
+            keychains: {
+              user: keychains[0],
+              backup: keychains[1],
+              bitgo: keychains[2],
+            },
+          },
+          reqId,
+        })
+      )
+    );
+
     const transactionParams = {
       ...params,
       keychain: keychains[0],
@@ -751,8 +772,6 @@ export class Wallet implements IWallet {
       allowNonSegwitSigningWithoutPrevTx: !!params.bulk,
     };
 
-    const txPrebuilds = Array.isArray(buildResponse) ? buildResponse : [buildResponse];
-
     const selectParams = _.pick(params, ['comment', 'otp', 'bulk']);
 
     const response = await Promise.all(