From 2ca0baf8753d4e8a5a91afe009fafc246110766c Mon Sep 17 00:00:00 2001 From: Veetrag Jain Date: Wed, 12 Aug 2026 19:35:17 +0530 Subject: [PATCH] fix(sdk-coin-stx): validate sbtcWithdrawParams in verifyTransaction verifyTransaction only checked txParams.recipients against explainTransaction outputs, sbtcWithdrawParams (amount, btcAddress, maxFee) was never actually verified. Add a type guard + dedicated verifySbtcWithdrawTransaction path that parses the raw tx via SbtcWithdrawBuilder and checks amount/maxFee/btcAddress against the expected params. TICKET: CSHLD-1451 --- modules/sdk-coin-stx/src/lib/index.ts | 2 + .../src/lib/sbtcWithdrawBuilder.ts | 29 +++++++ modules/sdk-coin-stx/src/stx.ts | 54 ++++++++++++ modules/sdk-coin-stx/test/unit/stx.ts | 85 ++++++++++++++++++- 4 files changed, 169 insertions(+), 1 deletion(-) diff --git a/modules/sdk-coin-stx/src/lib/index.ts b/modules/sdk-coin-stx/src/lib/index.ts index 612577631e..4850485801 100644 --- a/modules/sdk-coin-stx/src/lib/index.ts +++ b/modules/sdk-coin-stx/src/lib/index.ts @@ -3,4 +3,6 @@ export * from './keyPair'; export * from './transaction'; export * from './transactionBuilderFactory'; export * from './sbtcWithdrawBuilder'; +export * from './btcAddressUtils'; +export * from './iface'; export * as Utils from './utils'; diff --git a/modules/sdk-coin-stx/src/lib/sbtcWithdrawBuilder.ts b/modules/sdk-coin-stx/src/lib/sbtcWithdrawBuilder.ts index acba2336ba..974b0ec0a7 100644 --- a/modules/sdk-coin-stx/src/lib/sbtcWithdrawBuilder.ts +++ b/modules/sdk-coin-stx/src/lib/sbtcWithdrawBuilder.ts @@ -105,6 +105,35 @@ export class SbtcWithdrawBuilder extends AbstractContractBuilder { this._isDeserialized = true; } + /** + * Get the withdrawal params decoded from the deserialized/built transaction, including the + * raw sBTC recipient version and hash bytes (as opposed to a btcAddress string, which cannot + * be recovered from the on-chain args alone). + */ + getWithdrawParams(): + | { amount: string; maxFee: string; recipientVersion: number; recipientHashBytes: Buffer } + | undefined { + if (!this._withdrawParams) { + return undefined; + } + const payload = this.transaction.stxTransaction.payload as ContractCallPayload; + const recipientTuple = payload.functionArgs[1]; + if (recipientTuple?.type !== ClarityType.Tuple) { + return undefined; + } + const versionBuf = recipientTuple.data['version']; + const hashbytesBuf = recipientTuple.data['hashbytes']; + if (versionBuf?.type !== ClarityType.Buffer || hashbytesBuf?.type !== ClarityType.Buffer) { + return undefined; + } + return { + amount: this._withdrawParams.amount, + maxFee: this._withdrawParams.maxFee, + recipientVersion: versionBuf.buffer[0], + recipientHashBytes: Buffer.from(hashbytesBuf.buffer), + }; + } + /** @inheritdoc */ protected async buildImplementation(): Promise { if (!this._withdrawParams) { diff --git a/modules/sdk-coin-stx/src/stx.ts b/modules/sdk-coin-stx/src/stx.ts index 2b0abe0bd2..d51f242c78 100644 --- a/modules/sdk-coin-stx/src/stx.ts +++ b/modules/sdk-coin-stx/src/stx.ts @@ -11,6 +11,7 @@ import { MultisigType, multisigTypes, SignedTransaction, + TransactionParams, TransactionRecipient, TransactionType, VerifyAddressOptions, @@ -114,6 +115,11 @@ export class Stx extends BaseCoin { if (!rawTx) { throw new Error('missing required tx prebuild property txHex'); } + + if (this.hasSbtcWithdrawParams(txParams)) { + return this.verifySbtcWithdrawTransaction(rawTx, txParams.sbtcWithdrawParams); + } + const explainedTx = await this.explainTransaction({ txHex: rawTx, feeInfo: { fee: '' } }); const recipient = txParams.recipients?.[0]; if (recipient !== undefined && explainedTx) { @@ -149,6 +155,54 @@ export class Stx extends BaseCoin { return true; } + private hasSbtcWithdrawParams( + txParams: TransactionParams + ): txParams is TransactionParams & { sbtcWithdrawParams: StxLib.SbtcWithdrawParams } { + return 'sbtcWithdrawParams' in txParams && txParams.sbtcWithdrawParams !== undefined; + } + + /** + * Verify an sBTC withdrawal (burn) transaction matches the expected withdrawal params. + * + * @param rawTx - the raw (built) transaction hex from txPrebuild + * @param expected - the sbtcWithdrawParams supplied by the caller in txParams + */ + private async verifySbtcWithdrawTransaction(rawTx: string, expected: StxLib.SbtcWithdrawParams): Promise { + const factory = new StxLib.TransactionBuilderFactory(coins.get(this.getChain())); + const builder = factory.from(rawTx); + if (!(builder instanceof StxLib.SbtcWithdrawBuilder)) { + throw new Error('Tx is not a valid sBTC withdrawal transaction'); + } + + const actual = builder.getWithdrawParams(); + if (!actual) { + throw new Error('Unable to parse sBTC withdrawal params from tx'); + } + + if (BigInt(actual.amount) !== BigInt(expected.amount)) { + throw new Error( + `Tx sBTC withdrawal amount does not match expected amount: expected ${expected.amount} but got ${actual.amount}` + ); + } + if (BigInt(actual.maxFee) !== BigInt(expected.maxFee)) { + throw new Error( + `Tx sBTC withdrawal maxFee does not match expected maxFee: expected ${expected.maxFee} but got ${actual.maxFee}` + ); + } + + const decodedExpected = StxLib.decodeBtcAddress(expected.btcAddress); + if ( + decodedExpected.version !== actual.recipientVersion || + !decodedExpected.hashBytes.equals(actual.recipientHashBytes) + ) { + throw new Error( + `Tx sBTC withdrawal btcAddress does not match expected btcAddress: expected ${expected.btcAddress}` + ); + } + + return true; + } + /** * Check if address is valid, then make sure it matches the base address. * diff --git a/modules/sdk-coin-stx/test/unit/stx.ts b/modules/sdk-coin-stx/test/unit/stx.ts index df7d6903d1..347b26da88 100644 --- a/modules/sdk-coin-stx/test/unit/stx.ts +++ b/modules/sdk-coin-stx/test/unit/stx.ts @@ -5,9 +5,10 @@ import { BitGoAPI } from '@bitgo/sdk-api'; import { Wallet } from '@bitgo/sdk-core'; import { TestBitGo, TestBitGoAPI } from '@bitgo/sdk-test'; import { coins } from '@bitgo/statics'; -import { cvToString } from '@stacks/transactions'; +import { cvToString, pubKeyfromPrivKey, publicKeyToString } from '@stacks/transactions'; import * as testData from '../fixtures'; +import * as resources from './resources'; import { Stx, StxLib, Tstx } from '../../src'; import { RecoveryInfo, RecoveryOptions, RecoveryTransaction } from '../../src/lib/iface'; @@ -431,6 +432,88 @@ describe('STX:', function () { }); }); + describe('Verify sBTC Withdraw Transaction', function () { + const factory = new StxLib.TransactionBuilderFactory(coins.get('tstx')); + const prvKeysString = resources.prvKeysString.slice(0, 2); + + const buildWithdrawTx = async (withdrawParams: { amount: string; btcAddress: string; maxFee: string }) => { + const builder = factory.getSbtcWithdrawBuilder(); + builder.fee({ fee: '1000' }); + builder.nonce(1); + const pubKeys = prvKeysString.map((prv) => publicKeyToString(pubKeyfromPrivKey(prv))); + builder.fromPubKey(pubKeys); + builder.numberSignatures(2); + builder.withdraw(withdrawParams); + builder.sign({ key: prvKeysString[0] }); + builder.sign({ key: prvKeysString[1] }); + const tx = await builder.build(); + return tx.toBroadcastFormat(); + }; + + it('should succeed to verify a matching sBTC withdrawal', async function () { + const withdrawParams = { + amount: '1000', + btcAddress: 'bc1prxl88w47srqh703pxv567q47e7epzume4nlz8cgewfhtuenn8ngqgwm80w', + maxFee: '10000', + }; + const txHex = await buildWithdrawTx(withdrawParams); + const txParams = { + sbtcWithdrawParams: withdrawParams, + recipients: [{ address: 'SM1K9VF5GN48Q0AC2C7SB8WM5N5NR6DYC9VM3QEJE', amount: '10' }], + }; + const result = await basecoin.verifyTransaction({ txPrebuild: { txHex }, txParams }); + result.should.equal(true); + }); + + it('should fail to verify with wrong amount', async function () { + const withdrawParams = { + amount: '1000', + btcAddress: 'bc1prxl88w47srqh703pxv567q47e7epzume4nlz8cgewfhtuenn8ngqgwm80w', + maxFee: '10000', + }; + const txHex = await buildWithdrawTx(withdrawParams); + const txParams = { + sbtcWithdrawParams: { ...withdrawParams, amount: '9999' }, + recipients: [{ address: 'SM1K9VF5GN48Q0AC2C7SB8WM5N5NR6DYC9VM3QEJE', amount: '10' }], + }; + await basecoin + .verifyTransaction({ txPrebuild: { txHex }, txParams }) + .should.be.rejectedWith(/sBTC withdrawal amount does not match/); + }); + + it('should fail to verify with wrong maxFee', async function () { + const withdrawParams = { + amount: '1000', + btcAddress: 'bc1prxl88w47srqh703pxv567q47e7epzume4nlz8cgewfhtuenn8ngqgwm80w', + maxFee: '10000', + }; + const txHex = await buildWithdrawTx(withdrawParams); + const txParams = { + sbtcWithdrawParams: { ...withdrawParams, maxFee: '1' }, + recipients: [{ address: 'SM1K9VF5GN48Q0AC2C7SB8WM5N5NR6DYC9VM3QEJE', amount: '10' }], + }; + await basecoin + .verifyTransaction({ txPrebuild: { txHex }, txParams }) + .should.be.rejectedWith(/sBTC withdrawal maxFee does not match/); + }); + + it('should fail to verify with wrong btcAddress', async function () { + const withdrawParams = { + amount: '1000', + btcAddress: 'bc1prxl88w47srqh703pxv567q47e7epzume4nlz8cgewfhtuenn8ngqgwm80w', + maxFee: '10000', + }; + const txHex = await buildWithdrawTx(withdrawParams); + const txParams = { + sbtcWithdrawParams: { ...withdrawParams, btcAddress: '1BvBMSEYstWetqTFn5Au4m4GFg7xJaNVN2' }, + recipients: [{ address: 'SM1K9VF5GN48Q0AC2C7SB8WM5N5NR6DYC9VM3QEJE', amount: '10' }], + }; + await basecoin + .verifyTransaction({ txPrebuild: { txHex }, txParams }) + .should.be.rejectedWith(/sBTC withdrawal btcAddress does not match/); + }); + }); + describe('Recover Transaction STX', function () { before(function () { nock.enableNetConnect();