fix(sdk-core): add validation for unsigned txHex

Ticket: COIN-3222

TICKET: COIN-3222

Author: mullapudipruthvik

modules/abstract-cosmos/src/cosmosCoin.ts

@@ -351,10 +351,10 @@ export class CosmosCoin extends BaseCoin {
     if (!rawTx) {
       throw new Error('missing required tx prebuild property txHex');
     }
-    const transaction = await this.getBuilder().from(rawTx).build();
-    const explainedTx = transaction.explainTransaction();
 
     if (txParams.recipients && txParams.recipients.length > 0) {
+      const transaction = await this.getBuilder().from(rawTx).build();
+      const explainedTx = transaction.explainTransaction();
       const filteredRecipients = txParams.recipients?.map((recipient) => _.pick(recipient, ['address', 'amount']));
       const filteredOutputs = explainedTx.outputs.map((output) => _.pick(output, ['address', 'amount']));
 

modules/abstract-eth/src/abstractEthLikeNewCoins.ts

@@ -2299,13 +2299,13 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
    * @param {Wallet} params.wallet - Wallet object to obtain keys to verify against
    * @returns {boolean}
    */
-  verifyTssTransaction(params: VerifyEthTransactionOptions): boolean {
+  async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise<boolean> {
     const { txParams, txPrebuild, wallet } = params;
     if (
       !txParams?.recipients &&
       !(
         txParams.prebuildTx?.consolidateId ||
-        (txParams.type && ['acceleration', 'fillNonce', 'transferToken'].includes(txParams.type))
+        (txParams.type && ['acceleration', 'fillNonce', 'transferToken', 'transfer'].includes(txParams.type))
       )
     ) {
       throw new Error(`missing txParams`);
@@ -2316,6 +2316,28 @@ export abstract class AbstractEthLikeNewCoins extends AbstractEthLikeCoin {
     if (txParams.hop && txParams.recipients && txParams.recipients.length > 1) {
       throw new Error(`tx cannot be both a batch and hop transaction`);
     }
+
+    if (txParams.type && ['transfer', 'transferToken'].includes(txParams.type)) {
+      if (txParams.recipients && txParams.recipients.length > 0) {
+        const recipients = txParams.recipients;
+
+        if (recipients.length !== 1) {
+          throw new Error(`tss transaction only supports 1 recipient but ${recipients.length} found`);
+        }
+        const txBuilder = this.getTransactionBuilder();
+        txBuilder.from(txPrebuild.txHex);
+        const tx = await txBuilder.build();
+        const txJson = tx.toJson();
+        const expectedAmount = recipients[0].amount;
+        if (expectedAmount !== txJson.value) {
+          throw new Error('the transaction amount in txPrebuild does not match the value given by client');
+        }
+        if (recipients[0].address !== txJson.to) {
+          throw new Error('destination address does not match with the recipient address');
+        }
+      }
+    }
+
     return true;
   }
 

modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts

@@ -659,8 +659,11 @@ describe('TSS Ecdsa Utils:', async function () {
       transactions: [
         {
           unsignedTx: {
-            serializedTxHex: 'TOO MANY SECRETS',
-            signableHex: 'TOO MANY SECRETS',
+            // hteth txid: 0xc5a7bfe6b13ceae563da0f9feaa9c4ad1c101a15366a2a488828a5dd27cb9da3
+            serializedTxHex:
+              '02f38242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0808080',
+            signableHex:
+              '02f08242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0',
             derivationPath: '', // Needs this when key derivation is supported
           },
           state: 'pendingSignature',
@@ -669,8 +672,11 @@ describe('TSS Ecdsa Utils:', async function () {
       ],
       unsignedTxs: [
         {
-          serializedTxHex: 'TOO MANY SECRETS',
-          signableHex: 'TOO MANY SECRETS',
+          // hteth txid: 0xc5a7bfe6b13ceae563da0f9feaa9c4ad1c101a15366a2a488828a5dd27cb9da3
+          serializedTxHex:
+            '02f38242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0808080',
+          signableHex:
+            '02f38242688084448b9b8084448b9b908301637894a1cfb9d51c0af191ff21c5f0f01723e056f7dc12865af3107a400080c0808080',
           derivationPath: '', // Needs this when key derivation is supported
         },
       ],
@@ -933,6 +939,41 @@ describe('TSS Ecdsa Utils:', async function () {
       userGpgActual.should.startWith('-----BEGIN PGP PUBLIC KEY BLOCK-----');
     });
 
+    it('signTxRequest should fail with wrong recipient', async function () {
+      await setupSignTxRequestNocks(true, userSignShare, aShare, dShare, enterpriseData);
+      await tssUtils
+        .signTxRequest({
+          txRequest: txRequestId,
+          prv: JSON.stringify({
+            pShare: userKeyShare.pShare,
+            bitgoNShare: bitgoKeyShare.nShares[1],
+            backupNShare: backupKeyShare.nShares[1],
+          }),
+          reqId,
+          txParams: { recipients: [{ address: '0x1234', amount: '100000000000000' }], type: 'transfer' },
+        })
+        .should.be.rejectedWith('destination address does not match with the recipient address');
+    });
+
+    it('signTxRequest should fail with incorrect value', async function () {
+      await setupSignTxRequestNocks(true, userSignShare, aShare, dShare, enterpriseData);
+      await tssUtils
+        .signTxRequest({
+          txRequest: txRequestId,
+          prv: JSON.stringify({
+            pShare: userKeyShare.pShare,
+            bitgoNShare: bitgoKeyShare.nShares[1],
+            backupNShare: backupKeyShare.nShares[1],
+          }),
+          reqId,
+          txParams: {
+            recipients: [{ address: '0xa1cfb9d51c0af191ff21c5f0f01723e056f7dc12', amount: '1' }],
+            type: 'transfer',
+          },
+        })
+        .should.be.rejectedWith('the transaction amount in txPrebuild does not match the value given by client');
+    });
+
     it('getOfflineSignerPaillierModulus should succeed', async function () {
       const paillierModulus = tssUtils.getOfflineSignerPaillierModulus({
         prv: JSON.stringify({

modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts

@@ -152,7 +152,7 @@ export interface VerificationOptions {
 export interface VerifyTransactionOptions {
   txPrebuild: TransactionPrebuild;
   txParams: TransactionParams;
-  wallet: Wallet;
+  wallet: IWallet;
   verification?: VerificationOptions;
   reqId?: IRequestTracer;
   walletType?: 'onchain' | 'tss';

modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts

@@ -1,6 +1,6 @@
 import { Key, SerializedKeyPair } from 'openpgp';
 import { IRequestTracer } from '../../../api';
-import { KeychainsTriplet, ParsedTransaction } from '../../baseCoin';
+import { KeychainsTriplet, ParsedTransaction, TransactionParams } from '../../baseCoin';
 import { ApiKeyShare, Keychain } from '../../keychain';
 import { ApiVersion, Memo, WalletType } from '../../wallet';
 import { EDDSA, GShare, Signature, SignShare } from '../../../account-lib/mpc/tss';
@@ -422,6 +422,7 @@ export type TSSParams = {
   txRequest: string | TxRequest; // can be either a string or TxRequest
   reqId: IRequestTracer;
   apiVersion?: ApiVersion;
+  txParams?: TransactionParams;
 };
 
 export type TSSParamsForMessage = TSSParams & {

modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts

@@ -900,6 +900,12 @@ export class EcdsaUtils extends BaseEcdsaUtils {
       assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
       const unsignedTx =
         txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
+      await this.baseCoin.verifyTransaction({
+        txPrebuild: { txHex: unsignedTx.signableHex },
+        txParams: params.txParams || { recipients: [] },
+        wallet: this.wallet,
+        walletType: this.wallet.multisigType(),
+      });
       signablePayload = Buffer.from(unsignedTx.signableHex, 'hex');
       derivationPath = unsignedTx.derivationPath;
     } else if (requestType === RequestType.message) {

modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts

@@ -731,6 +731,12 @@ export class EcdsaMPCv2Utils extends BaseEcdsaUtils {
       assert(txRequest.transactions || txRequest.unsignedTxs, 'Unable to find transactions in txRequest');
       const unsignedTx =
         txRequest.apiVersion === 'full' ? txRequest.transactions![0].unsignedTx : txRequest.unsignedTxs[0];
+      await this.baseCoin.verifyTransaction({
+        txPrebuild: { txHex: unsignedTx.signableHex },
+        txParams: params.txParams || { recipients: [] },
+        wallet: this.wallet,
+        walletType: this.wallet.multisigType(),
+      });
       txOrMessageToSign = unsignedTx.signableHex;
       derivationPath = unsignedTx.derivationPath;
       bufferContent = Buffer.from(txOrMessageToSign, 'hex');

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

@@ -3426,6 +3426,7 @@ export class Wallet implements IWallet {
     try {
       return await this.tssUtils!.signTxRequest({
         txRequest: params.txPrebuild.txRequestId,
+        txParams: params.txPrebuild.buildParams,
         prv: params.prv,
         reqId: params.reqId || new RequestTracer(),
         apiVersion: params.apiVersion,