From 231e5f5adc82dd8e52999584d31d9b4581bd553c Mon Sep 17 00:00:00 2001 From: Bhuvan R Date: Wed, 13 May 2026 20:42:20 +0530 Subject: [PATCH] fix(sdk-coin-trx): validate TRC20 recipient and amount for TSS transactions TICKET: CHALO-448 --- modules/sdk-coin-trx/src/trx.ts | 37 +++-- modules/sdk-coin-trx/src/trxToken.ts | 58 ++++++- modules/sdk-coin-trx/test/unit/trxToken.ts | 147 ++++++++++++++++++ .../test/unit/verifyTransaction.ts | 31 +++- 4 files changed, 255 insertions(+), 18 deletions(-) create mode 100644 modules/sdk-coin-trx/test/unit/trxToken.ts diff --git a/modules/sdk-coin-trx/src/trx.ts b/modules/sdk-coin-trx/src/trx.ts index 3c19182542..e68383c1a7 100644 --- a/modules/sdk-coin-trx/src/trx.ts +++ b/modules/sdk-coin-trx/src/trx.ts @@ -389,15 +389,7 @@ export class Trx extends BaseCoin { // containing { txID, raw_data, raw_data_hex }. // 2. ECDSA signing flow (ecdsa.ts) — txHex is signableHex, the raw protobuf bytes (raw_data_hex). // We need to extract the raw_data_hex in case 1 before decoding. - let rawDataHex: string; - try { - // serializedTxHex: full JSON string — extract the raw_data_hex field - rawDataHex = JSON.parse(txPrebuild.txHex).raw_data_hex; - } catch { - // signableHex: already raw protobuf hex (raw_data_hex) - console.debug(`Could not parse txHex as JSON for coin ${this.getChain()}, using txHex directly`); - rawDataHex = txPrebuild.txHex; - } + const rawDataHex = this.extractRawDataHex(txPrebuild.txHex); const decodedTx = Utils.decodeTransaction(rawDataHex); // decodedTx uses a numeric enum for contract type (from protobuf decoding), @@ -411,6 +403,15 @@ export class Trx extends BaseCoin { return this.validateTransferContract(decodedTx.contract[0], txParams, true); } + if (decodedTx.contractType === Enum.ContractType.TriggerSmartContract) { + // TRC20 token transfers (TriggerSmartContract) must be verified via TrxToken.verifyTransaction, + // not here. Fail closed to prevent unvalidated token transfers from being silently signed. + throw new Error( + 'TriggerSmartContract verification is not supported by native TRX. ' + + 'TRC20 token transfers must be verified via TrxToken.verifyTransaction.' + ); + } + return true; } @@ -434,6 +435,24 @@ export class Trx extends BaseCoin { } } + /** + * Extract the raw protobuf hex (raw_data_hex) from a TSS txHex. + * + * TSS verifyTransaction is called from two places: + * 1. prebuildAndSignTransaction — txHex is a full JSON string containing raw_data_hex. + * 2. ECDSA signing flow — txHex is already the raw protobuf hex (raw_data_hex). + * + * This helper handles both formats so callers don't duplicate the try/catch. + */ + protected extractRawDataHex(txHex: string): string { + try { + return JSON.parse(txHex).raw_data_hex; + } catch { + console.debug(`Could not parse txHex as JSON for coin ${this.getChain()}, using txHex directly`); + return txHex; + } + } + /** * Validate Transfer contract (native TRX transfer). * Shared by both on-chain multisig and TSS wallet verification paths. diff --git a/modules/sdk-coin-trx/src/trxToken.ts b/modules/sdk-coin-trx/src/trxToken.ts index f91193c7f8..d824b9c16e 100644 --- a/modules/sdk-coin-trx/src/trxToken.ts +++ b/modules/sdk-coin-trx/src/trxToken.ts @@ -4,6 +4,7 @@ import { TrxTokenConfig, coins, tokens } from '@bitgo/statics'; import { getBuilder } from './lib/builder'; import { Recipient } from '../../sdk-core/src/bitgo/baseCoin/iBaseCoin'; import assert from 'assert'; +import { Enum, Utils, Interface } from './lib'; export { TrxTokenConfig }; @@ -94,15 +95,60 @@ export class TrxToken extends Trx { } async verifyTransaction(params: VerifyTransactionOptions): Promise { - const { txPrebuild: txPrebuild, txParams: txParams, walletType } = params; + const { txPrebuild, txParams, walletType } = params; assert(txPrebuild.txHex, new Error('missing required tx prebuild property txHex')); - // For TSS wallets, TRC20 token transfers are TriggerSmartContract transactions. - // Trx.verifyTransaction already returns true for TriggerSmartContract in TSS mode - // (only TransferContract/native TRX gets validated against recipients there). - // We apply the same convention here: the TSS signing protocol itself provides - // cryptographic guarantees; recipients-based verification is not applicable. if (walletType === 'tss') { + // For TSS wallets, TRC20 token transfers are TriggerSmartContract transactions. + // Decode the transaction and validate destination address and amount against + // txParams.recipients before signing to ensure intent matches the prebuild. + const rawDataHex = this.extractRawDataHex(txPrebuild.txHex); + const decodedTx = Utils.decodeTransaction(rawDataHex); + + if (decodedTx.contractType !== Enum.ContractType.TriggerSmartContract) { + throw new Error( + `Expected TriggerSmartContract for TRC20 token transfer, got contract type: ${decodedTx.contractType}` + ); + } + + if (!Array.isArray(decodedTx.contract) || decodedTx.contract.length !== 1) { + throw new Error('Invalid TriggerSmartContract structure'); + } + + const triggerContract = decodedTx.contract[0] as Interface.TriggerSmartContract; + // data is base64-encoded from protobuf decoding; convert to hex for decodeDataParams + const contractData = Buffer.from(triggerContract.parameter.value.data, 'base64').toString('hex'); + + const recipients = txParams.recipients || (txPrebuild.txInfo as TronTxInfo).recipients; + if (!recipients || recipients.length !== 1) { + throw new Error('missing or invalid required property recipients'); + } + + let recipientHex: string; + let transferAmount: { toString(): string }; + try { + [recipientHex, transferAmount] = Utils.decodeDataParams(['address', 'uint256'], contractData) as [ + string, + { toString(): string } + ]; + } catch (e) { + throw new Error(`Failed to decode TRC20 transfer ABI data: ${e instanceof Error ? e.message : String(e)}`); + } + + // recipientHex has '41' hex prefix; convert to base58 for comparison + const actualDestination = Utils.getBase58AddressFromHex(recipientHex); + const actualAmount = transferAmount.toString(); + const expectedDestination = recipients[0].address; + const expectedAmount = recipients[0].amount.toString(); + + if (actualAmount !== expectedAmount) { + throw new Error('transaction amount in txPrebuild does not match the value given by client'); + } + + if (expectedDestination.toLowerCase() !== actualDestination.toLowerCase()) { + throw new Error('destination address does not match with the recipient address'); + } + return true; } diff --git a/modules/sdk-coin-trx/test/unit/trxToken.ts b/modules/sdk-coin-trx/test/unit/trxToken.ts new file mode 100644 index 0000000000..37d1fc78eb --- /dev/null +++ b/modules/sdk-coin-trx/test/unit/trxToken.ts @@ -0,0 +1,147 @@ +import assert from 'node:assert'; +import { describe, it, before } from 'node:test'; +import { BitGoAPI } from '@bitgo/sdk-api'; +import { TestBitGoAPI, TestBitGo } from '@bitgo/sdk-test'; +import { tokens } from '@bitgo/statics'; +import { Trx } from '../../src/trx'; +import { TrxToken } from '../../src/trxToken'; +import { Ttrx } from '../../src/ttrx'; +import { Utils } from '../../src/lib'; + +// Real TriggerSmartContract protobuf raw_data_hex encoding a TRC20 transfer: +// owner: 41c51fbeea78910b15b1d3e8a9b62914ca94d1a4ac +// contract: 4142a1e39aefa49290f2b3f9ed688d7cecf86cd6e0 +// data: a9059cbb + abi(address=8483618ca85c35a9b923d98bebca718f5a1db279, uint256=100000000) +const TRC20_RAW_DATA_HEX = + '0a02578b22086113bb9ac351432b4088eae7a6de305aae01081f12a9010a31747970652e676f6f676c65617069732e636f6d2f70726f746f636f6c2e54726967676572536d617274436f6e747261637412740a1541c51fbeea78910b15b1d3e8a9b62914ca94d1a4ac12154142a1e39aefa49290f2b3f9ed688d7cecf86cd6e02244a9059cbb0000000000000000000000008483618ca85c35a9b923d98bebca718f5a1db2790000000000000000000000000000000000000000000000000000000005f5e10070888d8ca5de309001c0c39307'; + +// Recipient address decoded from the ABI data above (41 prefix → base58) +const TRC20_RECIPIENT_HEX = '418483618ca85c35a9b923d98bebca718f5a1db279'; +const TRC20_AMOUNT = '100000000'; + +describe('TrxToken verifyTransaction:', function () { + const bitgo: TestBitGoAPI = TestBitGo.decorate(BitGoAPI, { env: 'test' }); + bitgo.initializeTestVars(); + bitgo.safeRegister('trx', Trx.createInstance); + bitgo.safeRegister('ttrx', Ttrx.createInstance); + + let tokenCoin: TrxToken; + + before(function () { + const usdtConfig = tokens.testnet.trx.tokens.find((t) => t.type === 'ttrx:usdt'); + assert.ok(usdtConfig, 'ttrx:usdt token config not found'); + tokenCoin = new TrxToken(bitgo, usdtConfig); + }); + + describe('TSS wallet — TriggerSmartContract validation', () => { + it('should validate a correct TRC20 transfer', async function () { + const recipientBase58 = Utils.getBase58AddressFromHex(TRC20_RECIPIENT_HEX); + + const result = await tokenCoin.verifyTransaction({ + txPrebuild: { txHex: TRC20_RAW_DATA_HEX }, + txParams: { recipients: [{ address: recipientBase58, amount: TRC20_AMOUNT }] }, + walletType: 'tss', + } as any); + + assert.strictEqual(result, true); + }); + + it('should validate when txHex is a serialized JSON (prebuildAndSignTransaction path)', async function () { + const recipientBase58 = Utils.getBase58AddressFromHex(TRC20_RECIPIENT_HEX); + const serializedTxHex = JSON.stringify({ txID: 'abc', raw_data_hex: TRC20_RAW_DATA_HEX, raw_data: {} }); + + const result = await tokenCoin.verifyTransaction({ + txPrebuild: { txHex: serializedTxHex }, + txParams: { recipients: [{ address: recipientBase58, amount: TRC20_AMOUNT }] }, + walletType: 'tss', + } as any); + + assert.strictEqual(result, true); + }); + + it('should throw when amount does not match', async function () { + const recipientBase58 = Utils.getBase58AddressFromHex(TRC20_RECIPIENT_HEX); + + await assert.rejects( + tokenCoin.verifyTransaction({ + txPrebuild: { txHex: TRC20_RAW_DATA_HEX }, + txParams: { recipients: [{ address: recipientBase58, amount: '999' }] }, + walletType: 'tss', + } as any), + { message: 'transaction amount in txPrebuild does not match the value given by client' } + ); + }); + + it('should throw when recipient address does not match', async function () { + await assert.rejects( + tokenCoin.verifyTransaction({ + txPrebuild: { txHex: TRC20_RAW_DATA_HEX }, + txParams: { recipients: [{ address: 'TLWh67P93KgtnZNCtGnEHM1H33Nhq2uvvN', amount: TRC20_AMOUNT }] }, + walletType: 'tss', + } as any), + { message: 'destination address does not match with the recipient address' } + ); + }); + + it('should throw when recipients is empty', async function () { + await assert.rejects( + tokenCoin.verifyTransaction({ + txPrebuild: { txHex: TRC20_RAW_DATA_HEX }, + txParams: { recipients: [] }, + walletType: 'tss', + } as any), + { message: 'missing or invalid required property recipients' } + ); + }); + + it('should throw when contract type is not TriggerSmartContract', async function () { + // Use a native TRX Transfer protobuf as txHex — TrxToken only handles TriggerSmartContract + const recipientBase58 = Utils.getBase58AddressFromHex(TRC20_RECIPIENT_HEX); + const nativeTrxRawDataHex = Utils.generateRawDataHex({ + contract: [ + { + parameter: { + value: { + amount: 100000000, + owner_address: '4173a5993cd182ae152adad8203163f780c65a8aa5', + to_address: TRC20_RECIPIENT_HEX, + } as any, + type_url: 'type.googleapis.com/protocol.TransferContract', + }, + type: 'TransferContract', + } as any, + ], + refBlockBytes: 'c8cf', + refBlockHash: '89177fd84c5d9196', + expiration: Date.now() + 3600000, + timestamp: Date.now(), + }); + + await assert.rejects( + tokenCoin.verifyTransaction({ + txPrebuild: { txHex: nativeTrxRawDataHex }, + txParams: { recipients: [{ address: recipientBase58, amount: '100000000' }] }, + walletType: 'tss', + } as any), + { message: /Expected TriggerSmartContract for TRC20 token transfer/ } + ); + }); + }); + + describe('non-TSS wallet — builder-based validation (existing path)', () => { + it('should validate a correct non-TSS TRC20 transfer using txBuilder', async function () { + // The non-TSS path uses getBuilder().from(rawTx).build() and checks tx.outputs[0] + // This test uses the full JSON tx format that the builder understands. + const txHex = + '{"raw_data":{"contractType":2,"contract":[{"parameter":{"value":{"data":"a9059cbb0000000000000000000000008483618ca85c35a9b923d98bebca718f5a1db2790000000000000000000000000000000000000000000000000000000005f5e100","owner_address":"41c51fbeea78910b15b1d3e8a9b62914ca94d1a4ac","contract_address":"4142a1e39aefa49290f2b3f9ed688d7cecf86cd6e0"},"type_url":"type.googleapis.com/protocol.TriggerSmartContract"},"type":"TriggerSmartContract"}],"expiration":1674581767432,"timestamp":1674578167432,"ref_block_bytes":"578b","ref_block_hash":"6113bb9ac351432b","fee_limit":15000000},"raw_data_hex":"0a02578b22086113bb9ac351432b4088eae7a6de305aae01081f12a9010a31747970652e676f6f676c65617069732e636f6d2f70726f746f636f6c2e54726967676572536d617274436f6e747261637412740a1541c51fbeea78910b15b1d3e8a9b62914ca94d1a4ac12154142a1e39aefa49290f2b3f9ed688d7cecf86cd6e02244a9059cbb0000000000000000000000008483618ca85c35a9b923d98bebca718f5a1db2790000000000000000000000000000000000000000000000000000000005f5e10070888d8ca5de309001c0c39307","txID":"fe21c49f4febd9089125e3a006943c145721d8fcb7ab84136f8c6663ff92f8ed","signature":["0775cde302689eb8293883c66a89b31e80d608bfc3ad3c283b64a490ea4cc712c55a2fd2e62c75843dd7e77d8c4cb52e0f371fbb29b332c259f8cb63c2e6195301"]}'; + const recipientBase58 = Utils.getBase58AddressFromHex(TRC20_RECIPIENT_HEX); + + const result = await tokenCoin.verifyTransaction({ + txPrebuild: { txHex }, + txParams: { recipients: [{ address: recipientBase58, amount: TRC20_AMOUNT }] }, + } as any); + + assert.strictEqual(result, true); + }); + }); +}); diff --git a/modules/sdk-coin-trx/test/unit/verifyTransaction.ts b/modules/sdk-coin-trx/test/unit/verifyTransaction.ts index 188ce6553a..bd6173ff4d 100644 --- a/modules/sdk-coin-trx/test/unit/verifyTransaction.ts +++ b/modules/sdk-coin-trx/test/unit/verifyTransaction.ts @@ -540,9 +540,9 @@ describe('TRON Verify Transaction:', function () { }); }); - it('should return true for non-Transfer contract types in TSS', async function () { - // For non-Transfer contracts (e.g., AccountPermissionUpdate), TSS path returns true - // without detailed validation. + it('should return true for non-Transfer, non-TriggerSmartContract types in TSS', async function () { + // AccountPermissionUpdate and other native TRX contracts (freeze, vote, etc.) pass through + // without recipient validation in the TSS path. const rawDataHex = UnsignedAccountPermissionUpdateContractTx.raw_data_hex; const params = { @@ -560,6 +560,31 @@ describe('TRON Verify Transaction:', function () { assert.strictEqual(result, true); }); + it('should throw when TSS native TRX verifyTransaction encounters a TriggerSmartContract', async function () { + // Defense-in-depth: native TRX (Trx) should never verify TriggerSmartContract. + // TRC20 token transfers must go through TrxToken.verifyTransaction. + // The raw_data_hex below is a real TRC20 TriggerSmartContract protobuf. + const trc20RawDataHex = + '0a02578b22086113bb9ac351432b4088eae7a6de305aae01081f12a9010a31747970652e676f6f676c65617069732e636f6d2f70726f746f636f6c2e54726967676572536d617274436f6e747261637412740a1541c51fbeea78910b15b1d3e8a9b62914ca94d1a4ac12154142a1e39aefa49290f2b3f9ed688d7cecf86cd6e02244a9059cbb0000000000000000000000008483618ca85c35a9b923d98bebca718f5a1db2790000000000000000000000000000000000000000000000000000000005f5e10070888d8ca5de309001c0c39307'; + + const params = { + txParams: { + recipients: [{ address: 'TLWh67P93KgtnZNCtGnEHM1H33Nhq2uvvN', amount: '100000000' }], + }, + txPrebuild: { + txHex: trc20RawDataHex, + }, + wallet: {}, + walletType: 'tss', + }; + + await assert.rejects(basecoin.verifyTransaction(params), { + message: + 'TriggerSmartContract verification is not supported by native TRX. ' + + 'TRC20 token transfers must be verified via TrxToken.verifyTransaction.', + }); + }); + it('should throw error when txHex is missing for TSS wallet', async function () { const params = { txParams: {