fix: bind Xtz dataToSign to verified txHex before signing

modules/sdk-coin-xtz/package.json

@@ -46,6 +46,7 @@
     "@bitgo/statics": "^59.29.0",
     "@noble/curves": "1.8.1",
     "@taquito/local-forging": "^23.0.2",
+    "@taquito/michel-codec": "^23.0.3",
     "@taquito/rpc": "^23.0.2",
     "@taquito/signer": "^23.0.2",
     "bignumber.js": "^9.0.0",

modules/sdk-coin-xtz/src/lib/utils.ts

@@ -1,11 +1,12 @@
 import { isValidXpub, SigningError } from '@bitgo/sdk-core';
 import { InMemorySigner } from '@taquito/signer';
+import { packData } from '@taquito/michel-codec';
 import * as base58check from 'bs58check';
 import { secp256k1 } from '@noble/curves/secp256k1';
 import sodium from 'libsodium-wrappers';
-import { HashType, SignResponse } from './iface';
+import { HashType, SignResponse, TransactionOp } from './iface';
 import { KeyPair } from './keyPair';
-import { genericMultisigDataToSign } from './multisigUtils';
+import { genericMultisigDataToSign, getMultisigTransferDataFromOperation } from './multisigUtils';
 
 // By default, use the transactions prefix
 export const DEFAULT_WATERMARK = new Uint8Array([3]);
@@ -141,6 +142,56 @@ export function generateDataToSign(
   return genericMultisigDataToSign(contractAddress, destinationAddress, amount, contractCounter);
 }
 
+/**
+ * Pack the multisig transfer payload locally, producing the same bytes the Tezos node returns from
+ * /helpers/scripts/pack_data. This avoids trusting a node or the platform for the bytes that the
+ * wallet contract verifies signatures against.
+ *
+ * @param {string} contractAddress The wallet contract address with the funds to withdraw
+ * @param {string} destinationAddress The address to transfer the funds to
+ * @param {string} amount Number of mutez to transfer
+ * @param {string} contractCounter Wallet counter to use in the transaction
+ * @returns {string} The packed data to sign in hexadecimal format
+ */
+export function packGenericMultisigDataToSign(
+  contractAddress: string,
+  destinationAddress: string,
+  amount: string,
+  contractCounter: string
+): string {
+  const { data, type } = generateDataToSign(contractAddress, destinationAddress, amount, contractCounter);
+  return Buffer.from(packData(data, type)).toString('hex');
+}
+
+/**
+ * Fail unless the data to sign is exactly the payload authorizing the multisig transfer encoded in
+ * the given transaction operation. The signature over dataToSign is a standalone spend
+ * authorization for the wallet contract, so it must never be produced for bytes that are not
+ * derived from the transfer the user verified.
+ *
+ * @param {TransactionOp} transferOperation The multisig transfer operation decoded from the txHex
+ * @param {string} dataToSign The packed data to sign supplied alongside the txHex, in hexadecimal
+ * @throws {Error} if the operation is not a multisig transfer or the data to sign does not match it
+ */
+export function assertDataToSignMatchesTransfer(transferOperation: TransactionOp, dataToSign: string): void {
+  if (!transferOperation.parameters) {
+    throw new Error('Cannot validate dataToSign: the transaction is not a multisig transfer');
+  }
+  const transfer = getMultisigTransferDataFromOperation(transferOperation);
+  if (transfer.counter === undefined) {
+    throw new Error('Cannot validate dataToSign: the multisig transfer has no counter');
+  }
+  const expectedDataToSign = packGenericMultisigDataToSign(
+    transfer.from,
+    transfer.to,
+    transfer.amount,
+    transfer.counter
+  );
+  if (dataToSign.toLowerCase() !== expectedDataToSign) {
+    throw new Error('dataToSign does not match the transfer encoded in the transaction');
+  }
+}
+
 /**
  * Returns whether or not the string is a valid Tezos hash of the given type
  *

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

@@ -30,6 +30,7 @@ import { Interface, KeyPair, TransactionBuilder, Utils } from './lib';
 import { extractContractStorage, type ContractStorage } from './lib/multisigUtils';
 import { RecoverOptions } from './lib/iface';
 import {
+  assertDataToSignMatchesTransfer,
   generateDataToSign,
   isValidOriginatedAddress,
   TRANSACTION_FEE,
@@ -157,6 +158,15 @@ export class Xtz extends BaseCoin {
       throw new Error(`Tezos prebuild contains ${txOutputs.length} output(s) but expected exactly 1`);
     }
 
+    const prebuildDataToSign = (txPrebuild as Interface.TransactionPrebuild).dataToSign;
+    if (prebuildDataToSign) {
+      try {
+        await this.assertDataToSignBoundToTxHex(rawTx, prebuildDataToSign);
+      } catch (e) {
+        throw new Error(`Tezos prebuild dataToSign verification failed: ${e.message}`);
+      }
+    }
+
     const { address: prebuildAddress, value: prebuildAmount } = txOutputs[0];
 
     // Validate recipients if provided (normal send). Consolidation builds omit recipients.
@@ -200,6 +210,22 @@ export class Xtz extends BaseCoin {
     return derivedKeyNode.toBase58();
   }
 
+  /**
+   * Ensure a supplied dataToSign is derived from the transfer in txHex. The platform prebuild carries
+   * both artifacts independently, but only txHex is shown to the user for verification.
+   *
+   * @param {string} txHex The prebuilt transaction
+   * @param {string} dataToSign The packed multisig payload that would be signed
+   */
+  private async assertDataToSignBoundToTxHex(txHex: string, dataToSign: string): Promise<void> {
+    const txBuilder = new TransactionBuilder(coins.get(this.getChain()));
+    txBuilder.from(txHex);
+    const transaction = await txBuilder.build();
+    // The builder signs overridden dataToSign for the first operation, so that is the one it must match
+    const transferOperation = transaction.toJson().contents[0] as Interface.TransactionOp;
+    assertDataToSignMatchesTransfer(transferOperation, dataToSign);
+  }
+
   /**
    * Assemble keychain and half-sign prebuilt transaction
    *
@@ -209,6 +235,9 @@ export class Xtz extends BaseCoin {
    * @returns Bluebird<SignedTransaction>
    */
   async signTransaction(params: Interface.XtzSignTransactionOptions): Promise<SignedTransaction> {
+    if (params.txPrebuild.dataToSign) {
+      await this.assertDataToSignBoundToTxHex(params.txPrebuild.txHex, params.txPrebuild.dataToSign);
+    }
     const txBuilder = new TransactionBuilder(coins.get(this.getChain()));
     txBuilder.from(params.txPrebuild.txHex);
     txBuilder.source(params.txPrebuild.source);

modules/sdk-coin-xtz/test/fixtures.ts

@@ -1,13 +1,13 @@
 export const dataToSign =
-  '0507070a0000001601fe3263b84b615ee65c320d72a3d48fcb756fed580007070006050502000000510320053d036d0743036e0a0000001601619b12ee7d8808cf2434c1d8e432b5cf820a3415000555036c0200000015072f02000000090200000004034f032702000000000743036a0080897a034f034d031b';
+  '0507070a000000160100ac4a8356a8b7e3061c39e7797ebf5b5cbee0590007070000050502000000350320053d036d0743035d0a0000001501aaca87bdbcdc4e6117b667e29f9b504362c831bb031e0743036a0080b7a00b034f034d031b';
 export const unsignedHex =
   'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01622df81cc7ae34dd9d0346ba06198ee4de0dcae298f40202f093098102000100ac4a8356a8b7e3061c39e7797ebf5b5cbee05900ffff046d61696e0000005c070707070000050502000000440320053d036d0743035d0100000024747a3250744a397a67456746565452717936475873737435347448336b73456e59767653031e0743036a0080b7a00b034f034d031b0200000006030603060306';
 export const oneSignatureHex =
-  'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01622df81cc7ae34dd9d0346ba06198ee4de0dcae298f40202f093098102000100ac4a8356a8b7e3061c39e7797ebf5b5cbee05900ffff046d61696e000000c1070707070000050502000000440320053d036d0743035d0100000024747a3250744a397a67456746565452717936475873737435347448336b73456e59767653031e0743036a0080b7a00b034f034d031b020000006b050901000000607369677368313645716a6835334546396e62735937434d4b5458354675325439623736616a435563415a376b4d50394c42394854575173547a506e4e64635858534c6a393870614d54374337376a7939504e65675855484a796d77313167797703060306';
+  'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01622df81cc7ae34dd9d0346ba06198ee4de0dcae298f40202f093098102000100ac4a8356a8b7e3061c39e7797ebf5b5cbee05900ffff046d61696e000000c1070707070000050502000000440320053d036d0743035d0100000024747a3250744a397a67456746565452717936475873737435347448336b73456e59767653031e0743036a0080b7a00b034f034d031b020000006b05090100000060736967735970596f746375727768505863766975676e38626b566a677159326e533661664754593672576e663968314e4b55684a463165736a3547474765775353314b3257657a776e4a373731343835586570314132706a763554424b454b7003060306';
 export const twoSignatureHex =
-  'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01622df81cc7ae34dd9d0346ba06198ee4de0dcae298f40202f093098102000100ac4a8356a8b7e3061c39e7797ebf5b5cbee05900ffff046d61696e00000126070707070000050502000000440320053d036d0743035d0100000024747a3250744a397a67456746565452717936475873737435347448336b73456e59767653031e0743036a0080b7a00b034f034d031b02000000d0050901000000607369677368313645716a6835334546396e62735937434d4b5458354675325439623736616a435563415a376b4d50394c42394854575173547a506e4e64635858534c6a393870614d54374337376a7939504e65675855484a796d77313167797705090100000060736967504d463231434e5553534b755659356658546e437665694d456337767974654a36686257786834446a336a41644461473139757a4d725542797053595863585a5475373753566233376951546b4c6d4d35526b54385158394136344c560306';
+  'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01622df81cc7ae34dd9d0346ba06198ee4de0dcae298f40202f093098102000100ac4a8356a8b7e3061c39e7797ebf5b5cbee05900ffff046d61696e00000126070707070000050502000000440320053d036d0743035d0100000024747a3250744a397a67456746565452717936475873737435347448336b73456e59767653031e0743036a0080b7a00b034f034d031b02000000d005090100000060736967735970596f746375727768505863766975676e38626b566a677159326e533661664754593672576e663968314e4b55684a463165736a3547474765775353314b3257657a776e4a373731343835586570314132706a763554424b454b7005090100000060736967735970596f746375727768505863766975676e38626b566a677159326e533661664754593672576e663968314e4b55684a463165736a3547474765775353314b3257657a776e4a373731343835586570314132706a763554424b454b700306';
 export const fullySignedHex =
-  'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01622df81cc7ae34dd9d0346ba06198ee4de0dcae298f40202f093098102000100ac4a8356a8b7e3061c39e7797ebf5b5cbee05900ffff046d61696e00000126070707070000050502000000440320053d036d0743035d0100000024747a3250744a397a67456746565452717936475873737435347448336b73456e59767653031e0743036a0080b7a00b034f034d031b02000000d0050901000000607369677368313645716a6835334546396e62735937434d4b5458354675325439623736616a435563415a376b4d50394c42394854575173547a506e4e64635858534c6a393870614d54374337376a7939504e65675855484a796d77313167797705090100000060736967504d463231434e5553534b755659356658546e437665694d456337767974654a36686257786834446a336a41644461473139757a4d725542797053595863585a5475373753566233376951546b4c6d4d35526b54385158394136344c5603068dbc64a90f936e0796d4ec5f7010238a13a2ff45a69a689664de0e04833c3d4e5d7e1c0533943ea21a2b2f3c1df90a5438d05139dd531d95a6d3fbad852b8032';
+  'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01622df81cc7ae34dd9d0346ba06198ee4de0dcae298f40202f093098102000100ac4a8356a8b7e3061c39e7797ebf5b5cbee05900ffff046d61696e00000126070707070000050502000000440320053d036d0743035d0100000024747a3250744a397a67456746565452717936475873737435347448336b73456e59767653031e0743036a0080b7a00b034f034d031b02000000d005090100000060736967735970596f746375727768505863766975676e38626b566a677159326e533661664754593672576e663968314e4b55684a463165736a3547474765775353314b3257657a776e4a373731343835586570314132706a763554424b454b7005090100000060736967735970596f746375727768505863766975676e38626b566a677159326e533661664754593672576e663968314e4b55684a463165736a3547474765775353314b3257657a776e4a373731343835586570314132706a763554424b454b7003063f48c23ee031ff264a9b987998dc68ae75287eb57af4603dd06ad45e2feec8d4375f1b6c58ee19e25bab804df78b3887cc29339fdd548206917ae2e33518785a';
 
 export const unsignedTransactionWithTwoTransfersHex =
   'ba7a04fab1a3f77eda96b551947dd343e165d1b91b6f9f806648b63e57c88cc86c01aaca87bdbcdc4e6117b667e29f9b504362c831bb9c2500e8528102000196369c90625575ba44594b23794832a9337f7a2d00ffff046d61696e000000760707070700010505020000005e0320053d036d0743036e01000000244b543148557274366b66765979444559434a3247536a7654505a364b6d5266784c4255380555036c0200000015072f02000000090200000004034f032702000000000743036a00a401034f034d031b02000000060306030603066c01aaca87bdbcdc4e6117b667e29f9b504362c831bb9c2501e8528102000196369c90625575ba44594b23794832a9337f7a2d00ffff046d61696e0000005a070707070002050502000000420320053d036d0743035d0100000024747a3156526a5270564b6e76313641567072464831746b446e3454446656714138393341031e0743036a00a401034f034d031b0200000006030603060306';

modules/sdk-coin-xtz/test/unit/util.ts

@@ -10,6 +10,8 @@ import {
 } from '../resources';
 import { XtzLib } from '../../src';
 import { HashType } from '../../src/lib/iface';
+import { localForger } from '@taquito/local-forging';
+import { dataToSign, unsignedHex } from '../fixtures';
 
 describe('XTZ util library', function () {
   describe('address', function () {
@@ -238,6 +240,77 @@ describe('XTZ util library', function () {
     });
   });
 
+  describe('packGenericMultisigDataToSign', function () {
+    it('should pack locally to the same bytes the node packs', function () {
+      // The fixture dataToSign is the /pack_data RPC output for these exact parameters, so a local
+      // match proves packGenericMultisigDataToSign is byte-identical to the node.
+      const packed = XtzLib.Utils.packGenericMultisigDataToSign(
+        'KT18eKxkx71uMp28oZHjCiEj7QvFUaGzXtrZ',
+        'tz2PtJ9zgEgFVTRqy6GXsst54tH3ksEnYvvS',
+        '11800000',
+        '0'
+      );
+      packed.should.equal(dataToSign);
+    });
+  });
+
+  describe('assertDataToSignMatchesTransfer', function () {
+    // The operation under test is decoded from the unsignedHex fixture with the same local forger
+    // the transaction uses, so the multisig parameter shape is the production one.
+    const getTransferOperation = async () => {
+      const parsed = await localForger.parse(unsignedHex);
+      const operation = parsed.contents.find((content) => content.kind === 'transaction' && 'parameters' in content);
+      if (!operation) {
+        throw new Error('fixture has no multisig transaction operation');
+      }
+      return operation as Parameters<typeof XtzLib.Utils.assertDataToSignMatchesTransfer>[0];
+    };
+
+    it('should accept the locally packed payload for the transfer', async function () {
+      XtzLib.Utils.assertDataToSignMatchesTransfer(await getTransferOperation(), dataToSign);
+    });
+
+    it('should reject a payload that authorizes a different transfer', async function () {
+      // Flipping bytes yields a payload that is not derived from this transfer; accepting it would
+      // let a tampered prebuild be signed with the user key.
+      const tampered = dataToSign.slice(0, -4) + 'beef';
+      const operation = await getTransferOperation();
+      assert.throws(
+        () => XtzLib.Utils.assertDataToSignMatchesTransfer(operation, tampered),
+        new RegExp('dataToSign does not match the transfer encoded in the transaction')
+      );
+    });
+
+    it('should reject a non-multisig operation', async function () {
+      const parsed = await localForger.parse(unsignedHex);
+      const singlesigOperation = parsed.contents.find(
+        (content) => content.kind === 'transaction' && !('parameters' in content)
+      );
+      if (singlesigOperation === undefined) {
+        // unsignedHex has no singlesig op; pass an operation without parameters directly to
+        // exercise the same guard.
+        const emptyOperation = { kind: 'transaction', source: 'tz2HGMx8YxPSLguVP6usgf1D8UALPLchLoSX' };
+        assert.throws(
+          () =>
+            XtzLib.Utils.assertDataToSignMatchesTransfer(
+              emptyOperation as unknown as Parameters<typeof XtzLib.Utils.assertDataToSignMatchesTransfer>[0],
+              dataToSign
+            ),
+          new RegExp('not a multisig transfer')
+        );
+        return;
+      }
+      assert.throws(
+        () =>
+          XtzLib.Utils.assertDataToSignMatchesTransfer(
+            singlesigOperation as unknown as Parameters<typeof XtzLib.Utils.assertDataToSignMatchesTransfer>[0],
+            dataToSign
+          ),
+        new RegExp('not a multisig transfer')
+      );
+    });
+  });
+
   describe('signature', function () {
     it('should validate signature', function () {
       const validSignatures = [