diff --git a/modules/abstract-cosmos/src/cosmosCoin.ts b/modules/abstract-cosmos/src/cosmosCoin.ts index ec74813d91..6498a5c594 100644 --- a/modules/abstract-cosmos/src/cosmosCoin.ts +++ b/modules/abstract-cosmos/src/cosmosCoin.ts @@ -360,7 +360,6 @@ export class CosmosCoin extends BaseCoin { } const transaction = await this.getBuilder().from(rawTx).build(); const explainedTx = transaction.explainTransaction(); - if (txParams.recipients && txParams.recipients.length > 0) { const filteredRecipients = txParams.recipients?.map((recipient) => _.pick(recipient, ['address', 'amount'])); const filteredOutputs = explainedTx.outputs.map((output) => _.pick(output, ['address', 'amount'])); diff --git a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts index 321e71fd63..09f7da6fe2 100644 --- a/modules/abstract-eth/src/abstractEthLikeNewCoins.ts +++ b/modules/abstract-eth/src/abstractEthLikeNewCoins.ts @@ -66,8 +66,10 @@ import { calculateForwarderV1Address, ERC1155TransferBuilder, ERC721TransferBuilder, + getBufferedByteCode, getCommon, getProxyInitcode, + getRawDecoded, getToken, KeyPair as KeyPairLib, TransactionBuilder, @@ -2523,7 +2525,7 @@ 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 { const { txParams, txPrebuild, wallet } = params; if ( !txParams?.recipients && @@ -2540,6 +2542,39 @@ 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'].includes(txParams.type)) { + if (txParams.recipients && txParams.recipients.length === 1) { + const recipients = txParams.recipients; + const expectedAmount = recipients[0].amount.toString(); + const expectedDestination = recipients[0].address; + + const txBuilder = this.getTransactionBuilder(); + txBuilder.from(txPrebuild.txHex); + const tx = await txBuilder.build(); + const txJson = tx.toJson(); + if (txJson.data === '0x') { + if (expectedAmount !== txJson.value) { + throw new Error('the transaction amount in txPrebuild does not match the value given by client'); + } + if (expectedDestination !== txJson.to) { + throw new Error('destination address does not match with the recipient address'); + } + } else if (txJson.data.startsWith('0xa9059cbb')) { + const [recipientAddress, amount] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode('0xa9059cbb', txJson.data) + ); + if (expectedAmount !== amount.toString()) { + throw new Error('the transaction amount in txPrebuild does not match the value given by client'); + } + if (expectedDestination !== addHexPrefix(recipientAddress.toString())) { + throw new Error('destination address does not match with the recipient address'); + } + } + } + } + return true; } diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts index 9982ee5cb1..c5fc97c8de 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/ecdsa.ts +++ b/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,68 @@ 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('signTxRequest should fail with incorrect value for token txn', async function () { + const signableHex = + '02f86d8242681083122c9e83122cae8301e04994ebe8b46a42f05072b723b00013ff822b2af1b5cb80b844a9059cbb0000000000000000000000002b0d6cb2f8c388757f4d7ad857fccab18290dbc900000000000000000000000000000000000000000000000000000000000186a0c0'; + const serializedTxHex = + '02f8708242681083122c9e83122cae8301e04994ebe8b46a42f05072b723b00013ff822b2af1b5cb80b844a9059cbb0000000000000000000000002b0d6cb2f8c388757f4d7ad857fccab18290dbc900000000000000000000000000000000000000000000000000000000000186a0c0808080'; + await setupSignTxRequestNocks(true, userSignShare, aShare, dShare, enterpriseData, { + signableHex, + serializedTxHex, + apiVersion: 'full', + }); + await tssUtils + .signTxRequest({ + txRequest: txRequestId, + prv: JSON.stringify({ + pShare: userKeyShare.pShare, + bitgoNShare: bitgoKeyShare.nShares[1], + backupNShare: backupKeyShare.nShares[1], + }), + reqId, + txParams: { + recipients: [{ address: '0x2b0d6cb2f8c388757f4d7ad857fccab18290dbc9', amount: '707' }], + 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({ @@ -1101,7 +1169,12 @@ describe('TSS Ecdsa Utils:', async function () { userSignShare: ECDSA.SignShareRT, aShare: ECDSA.AShare, dShare: ECDSA.DShare, - enterpriseData?: EnterpriseData + enterpriseData: EnterpriseData, + { + signableHex, + serializedTxHex, + apiVersion, + }: { signableHex?: string; serializedTxHex?: string; apiVersion?: 'full' | 'lite' } = {} ) { if (enterpriseData) { await nockGetEnterprise({ enterpriseId: enterpriseData.id, response: enterpriseData, times: 1 }); @@ -1116,12 +1189,13 @@ describe('TSS Ecdsa Utils:', async function () { { ...txRequest, unsignedTx: { - signableHex: txRequest.unsignedTxs[0].signableHex, - serializedTxHex: txRequest.unsignedTxs[0].serializedTxHex, + signableHex: signableHex ?? txRequest.unsignedTxs[0].signableHex, + serializedTxHex: serializedTxHex ?? txRequest.unsignedTxs[0].serializedTxHex, derivationPath, }, }, ], + apiVersion: apiVersion, }, ], }; @@ -1145,6 +1219,7 @@ describe('TSS Ecdsa Utils:', async function () { }, }, ], + apiVersion: apiVersion, }, ], }; @@ -1165,6 +1240,7 @@ describe('TSS Ecdsa Utils:', async function () { }, }, ], + apiVersion: apiVersion, }, ], }; diff --git a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts index 6d30843a55..e2e60cb5b9 100644 --- a/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts +++ b/modules/sdk-core/src/bitgo/baseCoin/iBaseCoin.ts @@ -160,7 +160,7 @@ export interface VerificationOptions { export interface VerifyTransactionOptions { txPrebuild: TransactionPrebuild; txParams: TransactionParams; - wallet: Wallet; + wallet: IWallet; verification?: VerificationOptions; reqId?: IRequestTracer; walletType?: 'onchain' | 'tss'; diff --git a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts b/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts index 9f15f77afc..c644b9ba81 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/baseTypes.ts +++ b/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'; @@ -430,6 +430,7 @@ export type TSSParams = { txRequest: string | TxRequest; // can be either a string or TxRequest reqId: IRequestTracer; apiVersion?: ApiVersion; + txParams?: TransactionParams; }; export type TSSParamsForMessage = TSSParams & { 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 9f8ea67bae..e31195a7fd 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts +++ b/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) { 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 e27cefd925..9d2ca4bd7e 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts +++ b/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'); diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 46345bd2e4..19946ede1a 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -3441,6 +3441,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,