From 5ac3d861c78a917135daf24128f2d6bb3104262e Mon Sep 17 00:00:00 2001 From: BitGo Agent Date: Tue, 14 Jul 2026 07:01:18 +0000 Subject: [PATCH] fix(sdk-coin-xtz): verify destination and amount in verifyTransaction Previously, Xtz.verifyTransaction only checked that the recipient count was at most 1, then returned true unconditionally. This meant a compromised prebuild service could redirect an XTZ transfer to an attacker-controlled address and the local signer would still sign it. This fix decodes the transaction from txPrebuild.txHex when a single recipient is present and compares the encoded destination address and amount against txParams.recipients[0]. Any decode failure or mismatch throws, failing closed to prevent silent fund redirection. Behavior is unchanged when no recipients or no txHex are present (e.g. wallet initialization transactions), preserving backward compatibility. Ticket: CSHLD-838 Co-Authored-By: Claude Session-Id: 4815208a-ec3a-411f-a659-358c0aaa25e6 Task-Id: 0ce30aac-a75b-4b67-acca-38533b9d28f0 --- modules/sdk-coin-xtz/src/xtz.ts | 37 +++++++++++- modules/sdk-coin-xtz/test/unit/xtz.ts | 86 +++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 1 deletion(-) diff --git a/modules/sdk-coin-xtz/src/xtz.ts b/modules/sdk-coin-xtz/src/xtz.ts index 559b6b539e..056e6f68a6 100644 --- a/modules/sdk-coin-xtz/src/xtz.ts +++ b/modules/sdk-coin-xtz/src/xtz.ts @@ -122,12 +122,47 @@ export class Xtz extends BaseCoin { } async verifyTransaction(params: VerifyTransactionOptions): Promise { - const { txParams } = params; + const { txParams, txPrebuild } = params; if (Array.isArray(txParams.recipients) && txParams.recipients.length > 1) { throw new Error( `${this.getChain()} doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.` ); } + + const recipient = + Array.isArray(txParams.recipients) && txParams.recipients.length === 1 ? txParams.recipients[0] : undefined; + + // When a recipient is specified alongside a prebuild, decode the transaction + // and verify the encoded destination and amount match the requested recipient. + // Fail closed: any decode failure or mismatch is treated as a tampered prebuild. + if (recipient && txPrebuild?.txHex) { + let txOutputs: { address: string; value: string }[]; + try { + const txBuilder = new TransactionBuilder(coins.get(this.getChain())); + txBuilder.from(txPrebuild.txHex); + const tx = await txBuilder.build(); + txOutputs = tx.outputs; + } catch (e) { + throw new Error(`Failed to decode Tezos prebuild transaction: ${e.message}`); + } + + if (txOutputs.length !== 1) { + throw new Error(`Tezos prebuild contains ${txOutputs.length} output(s) but expected exactly 1`); + } + + const { address: prebuildAddress, value: prebuildAmount } = txOutputs[0]; + const requestedAddress = recipient.address; + const requestedAmount = recipient.amount.toString(); + + if (prebuildAddress !== requestedAddress) { + throw new Error(`Tezos prebuild destination mismatch: expected ${requestedAddress} but got ${prebuildAddress}`); + } + + if (new BigNumber(prebuildAmount).toFixed(0) !== new BigNumber(requestedAmount).toFixed(0)) { + throw new Error(`Tezos prebuild amount mismatch: expected ${requestedAmount} but got ${prebuildAmount}`); + } + } + return true; } diff --git a/modules/sdk-coin-xtz/test/unit/xtz.ts b/modules/sdk-coin-xtz/test/unit/xtz.ts index 61100dd70a..17baf70b58 100644 --- a/modules/sdk-coin-xtz/test/unit/xtz.ts +++ b/modules/sdk-coin-xtz/test/unit/xtz.ts @@ -204,6 +204,11 @@ describe('Tezos:', function () { describe('Verify Transaction', function () { const address1 = '5Ge59qRnZa8bxyhVFE6BDoY3kuhSrNVETRxXYLty1Hh6XTaf'; const address2 = '5DiMLZugmcKEH3igPZP367FqummZkWeW5Z6zDCHLfxRjnPXe'; + + // unsignedHex encodes: destination=tz2PtJ9zgEgFVTRqy6GXsst54tH3ksEnYvvS, amount=11800000 + const prebuildDestination = 'tz2PtJ9zgEgFVTRqy6GXsst54tH3ksEnYvvS'; + const prebuildAmount = '11800000'; + it('should reject a txPrebuild with more than one recipient', async function () { const wallet = new Wallet(bitgo, basecoin, {}); @@ -222,5 +227,86 @@ describe('Tezos:', function () { `txtz doesn't support sending to more than 1 destination address within a single transaction. Try again, using only a single recipient.` ); }); + + it('should verify a valid transaction where destination and amount match', async function () { + const txParams = { + recipients: [{ address: prebuildDestination, amount: prebuildAmount }], + }; + const txPrebuild = { txHex: unsignedHex }; + + const result = await basecoin.verifyTransaction({ txParams, txPrebuild }); + result.should.equal(true); + }); + + it('should reject when prebuild destination does not match requested recipient', async function () { + const txParams = { + recipients: [{ address: 'tz1VRjRpVKnv16AVprFH1tkDn4TDfVqA893A', amount: prebuildAmount }], + }; + const txPrebuild = { txHex: unsignedHex }; + + await basecoin + .verifyTransaction({ txParams, txPrebuild }) + .should.be.rejectedWith( + `Tezos prebuild destination mismatch: expected tz1VRjRpVKnv16AVprFH1tkDn4TDfVqA893A but got ${prebuildDestination}` + ); + }); + + it('should reject when prebuild amount does not match requested recipient', async function () { + const txParams = { + recipients: [{ address: prebuildDestination, amount: '99999999' }], + }; + const txPrebuild = { txHex: unsignedHex }; + + await basecoin + .verifyTransaction({ txParams, txPrebuild }) + .should.be.rejectedWith(`Tezos prebuild amount mismatch: expected 99999999 but got ${prebuildAmount}`); + }); + + it('should reject when the prebuild contains an unexpected number of outputs', async function () { + // unsignedTransactionWithTwoTransfersHex has 2 outputs, so it should fail the single-output check + const txParams = { + recipients: [{ address: prebuildDestination, amount: prebuildAmount }], + }; + const txPrebuild = { txHex: unsignedTransactionWithTwoTransfersHex }; + + await basecoin + .verifyTransaction({ txParams, txPrebuild }) + .should.be.rejectedWith('Tezos prebuild contains 2 output(s) but expected exactly 1'); + }); + + it('should reject when the prebuild cannot be decoded', async function () { + const txParams = { + recipients: [{ address: prebuildDestination, amount: prebuildAmount }], + }; + // A hex string that is not a valid Tezos transaction + const txPrebuild = { txHex: 'ff'.repeat(200) }; + + let threw = false; + try { + await basecoin.verifyTransaction({ txParams, txPrebuild }); + } catch (e) { + threw = true; + e.message.should.startWith('Failed to decode Tezos prebuild transaction'); + } + threw.should.equal(true); + }); + + it('should pass when no recipients are specified (no destination check)', async function () { + const txParams = { recipients: [] }; + const txPrebuild = { txHex: unsignedHex }; + + const result = await basecoin.verifyTransaction({ txParams, txPrebuild }); + result.should.equal(true); + }); + + it('should pass when no txHex is present in the prebuild (wallet init)', async function () { + const txParams = { + recipients: [{ address: prebuildDestination, amount: prebuildAmount }], + }; + const txPrebuild = {}; + + const result = await basecoin.verifyTransaction({ txParams, txPrebuild }); + result.should.equal(true); + }); }); });