diff --git a/modules/sdk-coin-bsc/src/bsc.ts b/modules/sdk-coin-bsc/src/bsc.ts index 9b3e255500..9eac4b6ba2 100644 --- a/modules/sdk-coin-bsc/src/bsc.ts +++ b/modules/sdk-coin-bsc/src/bsc.ts @@ -6,15 +6,21 @@ import { MultisigType, multisigTypes, NO_RECIPIENT_TX_TYPES, + Recipient, + TxIntentMismatchRecipientError, } from '@bitgo/sdk-core'; import { BaseCoin as StaticsBaseCoin, coins } from '@bitgo/statics'; import { AbstractEthLikeNewCoins, + getBufferedByteCode, + getRawDecoded, recoveryBlockchainExplorerQuery, VerifyEthTransactionOptions, } from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; +const addHexPrefix = (hex: string): string => (hex.startsWith('0x') ? hex : `0x${hex}`); + export class Bsc extends AbstractEthLikeNewCoins { protected constructor(bitgo: BitGoBase, staticsCoin?: Readonly) { super(bitgo, staticsCoin); @@ -64,20 +70,28 @@ export class Bsc extends AbstractEthLikeNewCoins { } /** - * Verify if a tss transaction is valid + * Verify if a tss transaction is valid. + * + * Performs the same 'transfer' calldata validation as AbstractEthLikeNewCoins: + * - native BNB (data === '0x'): checks destination address and amount + * - BEP-20 transfer() (0xa9059cbb): decodes calldata, checks destination and + * amount, including the WalletConnect recipients[0].data fallback * * @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} + * @returns {Promise} */ async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { const { txParams, txPrebuild, wallet } = params; + + const throwRecipientMismatch = (message: string, mismatchedRecipients: Recipient[]): never => { + throw new TxIntentMismatchRecipientError(message, undefined, [txParams], txPrebuild?.txHex, mismatchedRecipients); + }; + if ( !txParams?.recipients && !( txParams.prebuildTx?.consolidateId || + txPrebuild?.consolidateId || txParams.stakingRequestId || txParams.prebuildTx?.stakingRequestId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) @@ -92,6 +106,66 @@ export class Bsc extends AbstractEthLikeNewCoins { throw new Error(`tx cannot be both a batch and hop transaction`); } + if (txParams.type && txParams.type === 'transfer') { + 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) { + throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [ + { address: txJson.to, amount: txJson.value }, + ]); + } + if (expectedDestination.toLowerCase() !== txJson.to.toLowerCase()) { + throwRecipientMismatch('destination address does not match with the recipient address', [ + { address: txJson.to, amount: txJson.value }, + ]); + } + } else if (txJson.data.startsWith('0xa9059cbb')) { + const [recipientAddress, amount] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode('0xa9059cbb', txJson.data) + ); + + // Check if recipients[0].data exists (WalletConnect flow) + let expectedRecipientAddress: string; + let expectedTokenAmount: string; + const recipientData = (recipients[0] as any).data; + + if (recipientData && recipientData.startsWith('0xa9059cbb')) { + const [expectedRecipient, expectedAmt] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode('0xa9059cbb', recipientData) + ); + expectedRecipientAddress = addHexPrefix(expectedRecipient.toString()).toLowerCase(); + expectedTokenAmount = expectedAmt.toString(); + } else { + expectedRecipientAddress = expectedDestination.toLowerCase(); + expectedTokenAmount = expectedAmount; + } + + if (expectedTokenAmount !== amount.toString()) { + throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [ + { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() }, + ]); + } + + if (expectedRecipientAddress !== addHexPrefix(recipientAddress.toString()).toLowerCase()) { + throwRecipientMismatch('destination address does not match with the recipient address', [ + { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() }, + ]); + } + } + } + } + return true; } } diff --git a/modules/sdk-coin-bsc/src/bscToken.ts b/modules/sdk-coin-bsc/src/bscToken.ts index b2923cd137..f72b67321d 100644 --- a/modules/sdk-coin-bsc/src/bscToken.ts +++ b/modules/sdk-coin-bsc/src/bscToken.ts @@ -3,10 +3,26 @@ */ 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, + NO_RECIPIENT_TX_TYPES, + Recipient, + TxIntentMismatchRecipientError, +} from '@bitgo/sdk-core'; +import { + CoinNames, + EthLikeToken, + getBufferedByteCode, + getRawDecoded, + VerifyEthTransactionOptions, +} from '@bitgo/abstract-eth'; import { TransactionBuilder } from './lib'; +const addHexPrefix = (hex: string): string => (hex.startsWith('0x') ? hex : `0x${hex}`); + export { EthLikeTokenConfig }; export class BscToken extends EthLikeToken { @@ -43,21 +59,30 @@ export class BscToken extends EthLikeToken { getFullName(): string { return 'Bsc Token'; } + /** - * Verify if a tss transaction is valid + * Verify if a tss transaction is valid. + * + * Performs the same 'transfer' calldata validation as AbstractEthLikeNewCoins: + * - native transfer (data === '0x'): checks destination address and amount + * - BEP-20 transfer() (0xa9059cbb): decodes calldata, checks destination and + * amount, including the WalletConnect recipients[0].data fallback * * @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} + * @returns {Promise} */ async verifyTssTransaction(params: VerifyEthTransactionOptions): Promise { const { txParams, txPrebuild, wallet } = params; + + const throwRecipientMismatch = (message: string, mismatchedRecipients: Recipient[]): never => { + throw new TxIntentMismatchRecipientError(message, undefined, [txParams], txPrebuild?.txHex, mismatchedRecipients); + }; + if ( !txParams?.recipients && !( txParams.prebuildTx?.consolidateId || + txPrebuild?.consolidateId || txParams.stakingRequestId || txParams.prebuildTx?.stakingRequestId || (txParams.type && NO_RECIPIENT_TX_TYPES.has(txParams.type)) @@ -72,6 +97,66 @@ export class BscToken extends EthLikeToken { throw new Error(`tx cannot be both a batch and hop transaction`); } + if (txParams.type && txParams.type === 'transfer') { + 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) { + throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [ + { address: txJson.to, amount: txJson.value }, + ]); + } + if (expectedDestination.toLowerCase() !== txJson.to.toLowerCase()) { + throwRecipientMismatch('destination address does not match with the recipient address', [ + { address: txJson.to, amount: txJson.value }, + ]); + } + } else if (txJson.data.startsWith('0xa9059cbb')) { + const [recipientAddress, amount] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode('0xa9059cbb', txJson.data) + ); + + // Check if recipients[0].data exists (WalletConnect flow) + let expectedRecipientAddress: string; + let expectedTokenAmount: string; + const recipientData = (recipients[0] as any).data; + + if (recipientData && recipientData.startsWith('0xa9059cbb')) { + const [expectedRecipient, expectedAmt] = getRawDecoded( + ['address', 'uint256'], + getBufferedByteCode('0xa9059cbb', recipientData) + ); + expectedRecipientAddress = addHexPrefix(expectedRecipient.toString()).toLowerCase(); + expectedTokenAmount = expectedAmt.toString(); + } else { + expectedRecipientAddress = expectedDestination.toLowerCase(); + expectedTokenAmount = expectedAmount; + } + + if (expectedTokenAmount !== amount.toString()) { + throwRecipientMismatch('the transaction amount in txPrebuild does not match the value given by client', [ + { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() }, + ]); + } + + if (expectedRecipientAddress !== addHexPrefix(recipientAddress.toString()).toLowerCase()) { + throwRecipientMismatch('destination address does not match with the recipient address', [ + { address: addHexPrefix(recipientAddress.toString()), amount: amount.toString() }, + ]); + } + } + } + } + return true; } } diff --git a/modules/sdk-coin-bsc/test/unit/bsc.ts b/modules/sdk-coin-bsc/test/unit/bsc.ts index 6c80d6047b..95f1e16aa9 100644 --- a/modules/sdk-coin-bsc/test/unit/bsc.ts +++ b/modules/sdk-coin-bsc/test/unit/bsc.ts @@ -2,8 +2,12 @@ import 'should'; import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { BitGoAPI } from '@bitgo/sdk-api'; +import { TransactionType, Wallet } from '@bitgo/sdk-core'; +import EthereumAbi from 'ethereumjs-abi'; import { Bsc, Tbsc } from '../../src/index'; +import { TransactionBuilder } from '../../src/lib'; +import { getBuilder } from './getBuilder'; const bitgo: TestBitGoAPI = TestBitGo.decorate(BitGoAPI, { env: 'test' }); @@ -39,4 +43,187 @@ 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 methodId = EthereumAbi.methodID('transfer', ['address', 'uint256']); + const encodedParams = EthereumAbi.rawEncode(['address', 'uint256'], [recipientAddress, '10000000']); + const erc20TransferData = '0x' + Buffer.concat([methodId, encodedParams]).toString('hex'); + + 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 methodId = EthereumAbi.methodID('transfer', ['address', 'uint256']); + const encodedParams = EthereumAbi.rawEncode(['address', 'uint256'], [wrongAddress, '10000000']); + const erc20TransferData = '0x' + Buffer.concat([methodId, encodedParams]).toString('hex'); + + 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 methodId = EthereumAbi.methodID('transfer', ['address', 'uint256']); + const encodedParams = EthereumAbi.rawEncode(['address', 'uint256'], [recipientAddress, '10000000']); + const erc20TransferData = '0x' + Buffer.concat([methodId, encodedParams]).toString('hex'); + + 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 methodId = EthereumAbi.methodID('transfer', ['address', 'uint256']); + const tamperedParams = EthereumAbi.rawEncode(['address', 'uint256'], [wrongAddress, '10000000']); + const tamperedData = '0x' + Buffer.concat([methodId, tamperedParams]).toString('hex'); + + 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 correctParams = EthereumAbi.rawEncode(['address', 'uint256'], [recipientAddress, '10000000']); + const correctData = '0x' + Buffer.concat([methodId, correctParams]).toString('hex'); + + 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'); + }); + }); });