diff --git a/modules/bitgo/test/v2/unit/internal/tssUtils/eddsaMPCv2/signTxRequest.ts b/modules/bitgo/test/v2/unit/internal/tssUtils/eddsaMPCv2/signTxRequest.ts index e5403c3f4d..5c856df5ca 100644 --- a/modules/bitgo/test/v2/unit/internal/tssUtils/eddsaMPCv2/signTxRequest.ts +++ b/modules/bitgo/test/v2/unit/internal/tssUtils/eddsaMPCv2/signTxRequest.ts @@ -1,9 +1,11 @@ +import * as sinon from 'sinon'; import { BaseCoin, BitgoGPGPublicKey, common, ECDSAUtils, EDDSAUtils, + InvalidTransactionError, RequestTracer, RequestType, SignatureShareRecord, @@ -419,6 +421,134 @@ describe('signTxRequest:', function () { nockPromises[3].isDone().should.be.false(); }); + + describe('resolveEffectiveTxParams guard (WCI-1111)', function () { + let sandbox: sinon.SinonSandbox; + + beforeEach(function () { + sandbox = sinon.createSandbox(); + }); + + afterEach(function () { + sandbox.restore(); + }); + + it('throws InvalidTransactionError when txParams is absent and intent has no recipients (malicious/empty-recipient path)', async function () { + // Simulate the stakingAuthorize attack vector: intent has no recipients + // and intentType is not on the NO_RECIPIENT_TX_TYPES allowlist. + const maliciousTxRequest: TxRequest = { + ...txRequest, + intent: { intentType: 'stakingAuthorize' } as any, + }; + const userPrvBase64 = Buffer.from(userKeyShare).toString('base64'); + await tssUtils + .signTxRequest({ + txRequest: maliciousTxRequest, + prv: userPrvBase64, + reqId, + // No txParams — the re-sign path that was previously vulnerable + }) + .should.be.rejectedWith(InvalidTransactionError); + }); + + it('throws InvalidTransactionError when txParams has empty recipients and intentType is not allowlisted', async function () { + const maliciousTxRequest: TxRequest = { + ...txRequest, + intent: { intentType: 'payment' } as any, + }; + const userPrvBase64 = Buffer.from(userKeyShare).toString('base64'); + await tssUtils + .signTxRequest({ + txRequest: maliciousTxRequest, + prv: userPrvBase64, + reqId, + txParams: { recipients: [] }, + }) + .should.be.rejectedWith(InvalidTransactionError); + }); + + it('does not throw for allowlisted no-recipient intentType (deactivate)', async function () { + sandbox.stub(baseCoin, 'verifyTransaction').resolves(true); + const nockPromises = await getNockPromisesForEddsaSigning(txRequest); + await Promise.all(nockPromises); + + const noRecipientTxRequest: TxRequest = { + ...txRequest, + intent: { intentType: 'deactivate' } as any, + }; + const userPrvBase64 = Buffer.from(userKeyShare).toString('base64'); + await tssUtils.signTxRequest({ + txRequest: noRecipientTxRequest, + prv: userPrvBase64, + reqId, + // No txParams — legitimate no-recipient flow + }); + }); + + it('does not throw for allowlisted no-recipient intentType (consolidate)', async function () { + sandbox.stub(baseCoin, 'verifyTransaction').resolves(true); + const nockPromises = await getNockPromisesForEddsaSigning(txRequest); + await Promise.all(nockPromises); + + const consolidateTxRequest: TxRequest = { + ...txRequest, + intent: { intentType: 'consolidate' } as any, + }; + const userPrvBase64 = Buffer.from(userKeyShare).toString('base64'); + await tssUtils.signTxRequest({ + txRequest: consolidateTxRequest, + prv: userPrvBase64, + reqId, + }); + }); + + it('uses intent recipients when txParams is absent and intent has recipients', async function () { + sandbox.stub(baseCoin, 'verifyTransaction').resolves(true); + const nockPromises = await getNockPromisesForEddsaSigning(txRequest); + await Promise.all(nockPromises); + + const intentRecipientTxRequest: TxRequest = { + ...txRequest, + intent: { + intentType: 'payment', + recipients: [ + { + address: { address: 'HMEgbR4S2hLKfst2VZUVpHVUu4FioFPyW5iUuJvZdMvs' }, + amount: { value: '999990000', symbol: 'sol' }, + }, + ], + } as any, + }; + const userPrvBase64 = Buffer.from(userKeyShare).toString('base64'); + // Should not throw — intent provides the recipients + await tssUtils.signTxRequest({ + txRequest: intentRecipientTxRequest, + prv: userPrvBase64, + reqId, + }); + }); + + it('does not throw for staking intent with stakingRequestId (generic staking signal)', async function () { + sandbox.stub(baseCoin, 'verifyTransaction').resolves(true); + const nockPromises = await getNockPromisesForEddsaSigning(txRequest); + await Promise.all(nockPromises); + + const stakingTxRequest: TxRequest = { + ...txRequest, + intent: { + intentType: 'delegate', + stakingRequestId: 'staking-req-id-123', + } as any, + }; + const userPrvBase64 = Buffer.from(userKeyShare).toString('base64'); + await tssUtils.signTxRequest({ + txRequest: stakingTxRequest, + prv: userPrvBase64, + reqId, + }); + }); + }); + async function getNockPromisesForEddsaSigning( txRequest: TxRequest, requestType: RequestType = RequestType.tx, diff --git a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts index 311262515c..9613906817 100644 --- a/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts +++ b/modules/sdk-core/src/bitgo/utils/tss/eddsa/eddsaMPCv2.ts @@ -47,6 +47,7 @@ import { import { EncryptionVersion } from '../../../../api'; import { BitGoBase } from '../../../bitgoBase'; import { BaseEddsaUtils } from './base'; +import { resolveEffectiveTxParams } from '../recipientUtils'; import { EddsaMPCv2KeyGenSendFn, KeyGenSenderForEnterprise } from './eddsaMPCv2KeyGenSender'; import { EddsaMPCv2RecoveryKeyShares } from './types'; @@ -553,7 +554,7 @@ export class EddsaMPCv2Utils extends BaseEddsaUtils { bufferContent = Buffer.from(txOrMessageToSign, 'hex'); await this.baseCoin.verifyTransaction({ txPrebuild: { txHex: unsignedTx.serializedTxHex ?? txOrMessageToSign }, - txParams: params.txParams || { recipients: [] }, + txParams: resolveEffectiveTxParams(txRequest, params.txParams, this.baseCoin.getChain()), wallet: this.wallet, walletType: this.wallet.multisigType(), }); diff --git a/modules/sdk-core/src/bitgo/wallet/wallet.ts b/modules/sdk-core/src/bitgo/wallet/wallet.ts index 6c5babf9f4..3d372128ba 100644 --- a/modules/sdk-core/src/bitgo/wallet/wallet.ts +++ b/modules/sdk-core/src/bitgo/wallet/wallet.ts @@ -54,7 +54,6 @@ import { } from '../utils'; import { decodeWithCodec } from '../utils/codecs'; import { postWithCodec } from '../utils/postWithCodec'; -import { txParamsFromIntent } from '../utils/tss/baseTSSUtils'; import { EcdsaMPCv2Utils, EcdsaUtils } from '../utils/tss/ecdsa'; import EddsaUtils, { EddsaMPCv2Utils } from '../utils/tss/eddsa'; import { getTxRequestApiVersion, validateTxRequestApiVersion } from '../utils/txRequest'; @@ -5059,19 +5058,6 @@ export class Wallet implements IWallet { let txRequest: string | TxRequest = params.txPrebuild.txRequestId; let txParams: TransactionParams | undefined = params.txPrebuild.buildParams; - // EdDSA MPCv2 re-sign path: buildParams is absent when the UI calls signAndSendTxRequest with - // only txRequestId. Derive txParams from the persisted intent so verifyTransaction receives - // the correct recipients before DSG starts. Other TSS variants are unaffected by the guard. - if (!txParams && this.multisigTypeVersion() === 'MPCv2' && this.baseCoin.getMPCAlgorithm() === 'eddsa') { - txRequest = await getTxRequest( - this.bitgo, - this.id(), - params.txPrebuild.txRequestId, - params.reqId || new RequestTracer() - ); - txParams = txParamsFromIntent(txRequest.intent, this.baseCoin.getChain()); - } - try { return await this.tssUtils!.signTxRequest({ txRequest,