fix(sdk-coin-trx): validate TRC20 recipient and amount for TSS transctions#8767
Open
bhuvanr159 wants to merge 1 commit into
Open
fix(sdk-coin-trx): validate TRC20 recipient and amount for TSS transctions#8767bhuvanr159 wants to merge 1 commit into
bhuvanr159 wants to merge 1 commit into
Conversation
2759a98 to
58d2f07
Compare
Contributor
|
@claude review |
at31416
reviewed
May 14, 2026
| 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) { |
Contributor
There was a problem hiding this comment.
will a trc20 transaction initiated by bitgo will always have a single recipient? if at all recipient is present.
Contributor
Author
There was a problem hiding this comment.
Yes, a TRC20 transfer on Tron will always have exactly one recipient per transaction. The ABI signature transfer(address, uint256) only accepts one destination address and one amount, it's enforced by the protocol itself.
at31416
reviewed
May 14, 2026
| throw new Error('missing or invalid required property recipients'); | ||
| } | ||
|
|
||
| let decodedParams: any[]; |
Contributor
There was a problem hiding this comment.
can the usage of any be avoided?
…ctions TICKET: CHALO-448
58d2f07 to
231e5f5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TRX TSS token verification was unconditionally returning
truebefore validating any recipient or amount for TRC20 transfers. This meant a compromised BitGo platform or substituted prebuild couldredirect a TRC20 transfer (e.g. USDT-TRON) and the client SDK would still sign it without detecting the mismatch.
There are two bugs, one compounding the other:
Bug 1 —
trxToken.ts:105-107(primary)TrxToken.verifyTransactionshort-circuited toreturn truefor any TSS wallet, skipping all recipient and amount validation. The comment justified this by claiming "the TSS signing protocol itselfprovides cryptographic guarantees" — but TSS only guarantees the math (no single party can sign alone). It does not guarantee that what's being signed matches the user's intent. Intent verification is a
completely separate layer and is exactly what
verifyTransactionexists to do.Bug 2 —
trx.ts:414(defense-in-depth)Even if Bug 1 didn't exist, the parent
Trx.verifyTransactioncorrectly validates native TRXTransfercontracts but falls through withreturn trueforTriggerSmartContract— the protobuf typeused for all TRC20 transfers. So there was no safety net.
What this PR does
Removes the TSS shortcut in
TrxToken.verifyTransactionand replaces it with real validation: decodes theTriggerSmartContractprotobuf, extracts the ABI-encodedtransfer(address, uint256)call data using the existing
Utils.decodeDataParamsutility, converts the decoded address to base58, and compares both destination and amount againsttxParams.recipientsbefore signing. Fails closedon any decode failure or mismatch.
Adds a hard failure in
Trx.verifyTransactionforTriggerSmartContractin TSS mode. Native TRX should never receive a TRC20 transaction — if it does, we now throw instead of silently returningtrue. Other non-Transfer contract types (freeze, vote, delegate, etc.) are legitimate native TRX operations and continue to pass through.Adds unit tests for the
TrxTokenTSS verification path covering: valid transfer, JSONtxHexformat (prebuildAndSignTransaction path), amount mismatch, address mismatch, empty recipients, wrongcontract type, and the existing non-TSS builder path. Also adds a test asserting native
Trxnow throws onTriggerSmartContractin TSS mode.No new utilities needed
All the building blocks already existed in the codebase:
Utils.decodeTransaction(rawDataHex)— used intrx.tsto decode the protobufUtils.decodeDataParams(['address', 'uint256'], data)— used intransaction.tsto decode TRC20 ABI dataUtils.getBase58AddressFromHex(hex)— used intransaction.tsto convert the decoded addressImpact
All TSS Tron TRC20 transactions, including high-volume assets such as USDT-TRON, were affected. This fix ensures recipient and amount are verified against client-supplied
txParamsbefore the signingprotocol proceeds.
Ticket: CHALO-349
Parent: WAL-1449