Skip to content

fix(tss): require txParams with recipients for TSS tx signing#8462

Merged
mrdanish26 merged 1 commit intomasterfrom
WAL-375-mpcv2-sign-tx-require-txparams
Apr 14, 2026
Merged

fix(tss): require txParams with recipients for TSS tx signing#8462
mrdanish26 merged 1 commit intomasterfrom
WAL-375-mpcv2-sign-tx-require-txparams

Conversation

@mrdanish26
Copy link
Copy Markdown
Contributor

@mrdanish26 mrdanish26 commented Apr 9, 2026 •
edited
Loading

Summary

  • Fixes WAL-375: MPCv2 (and ECDSA TSS) signTxRequest() silently defaulted txParams to { recipients: [] } when the caller omitted it, allowing a compromised BitGo API to swap signableHex to redirect funds without client detection
  • Adds an early guard in signRequestBase() that throws when txParams.recipients is absent or empty for RequestType.tx, ensuring verifyTransaction() always receives explicit caller-supplied params
  • Propagates optional txParams through recreateTxRequest() so the pending-approval re-sign path keeps working; extracts recipients from pendingApproval.info.transactionRequest

Test plan

  • Existing success tests for signTxRequest in ecdsaMPCv2/signTxRequest.ts and ecdsa.ts updated to pass txParams with recipients
  • New negative tests verify rejection when txParams is missing
  • New negative tests verify rejection when txParams.recipients is an empty array
  • Run yarn unit-test in modules/bitgo

@linear
Copy link
Copy Markdown

linear bot commented Apr 9, 2026

WAL-375 [Security] MPCv2 signTxRequest() Defaults txParams to Empty Recipients — Transaction Verification Bypass

@mrdanish26 mrdanish26 force-pushed the WAL-375-mpcv2-sign-tx-require-txparams branch from 63ddf05 to 346f349 Compare April 9, 2026 19:12
@bitgo-ai-agent-dev bitgo-ai-agent-dev bot force-pushed the WAL-375-mpcv2-sign-tx-require-txparams branch from 346f349 to b04cdf0 Compare April 9, 2026 19:38
@mrdanish26 mrdanish26 marked this pull request as ready for review April 9, 2026 22:59
@mrdanish26 mrdanish26 requested review from a team as code owners April 9, 2026 22:59
@sachushaji
Copy link
Copy Markdown
Contributor

sachushaji commented Apr 10, 2026

@claude

pranavjain97
pranavjain97 requested changes Apr 10, 2026
View reviewed changes
Comment thread modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsa.ts Outdated
Comment thread modules/sdk-core/src/bitgo/utils/tss/ecdsa/ecdsaMPCv2.ts
Comment thread modules/sdk-core/src/bitgo/pendingApproval/pendingApproval.ts
@mrdanish26 mrdanish26 force-pushed the WAL-375-mpcv2-sign-tx-require-txparams branch 2 times, most recently from 2245b20 to ab5753e Compare April 10, 2026 18:59
@mrdanish26 mrdanish26 force-pushed the WAL-375-mpcv2-sign-tx-require-txparams branch from ab5753e to 2be89f5 Compare April 10, 2026 19:03
@mrdanish26 mrdanish26 requested a review from pranavjain97 April 10, 2026 19:17
zhongxishen
zhongxishen approved these changes Apr 14, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@zhongxishen zhongxishen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mrdanish26 mrdanish26 merged commit 1b9c75b into master Apr 14, 2026
22 checks passed
mrdanish26 added a commit that referenced this pull request Apr 16, 2026
@mrdanish26
Revert "Merge pull request #8462 from BitGo/WAL-375-mpcv2-sign-tx-req…
96ba3bb 
…uire-txparams"

This reverts commit 1b9c75b, reversing
changes made to 26a1c8e.
@mrdanish26 mrdanish26 mentioned this pull request Apr 16, 2026
Merged
mrdanish26 added a commit that referenced this pull request Apr 16, 2026
@mrdanish26
Revert "#8462 from BWAL-375-mpcv2-sign-tx-require-txparams"
dfbcd47 
This reverts commit 1b9c75b, reversing
changes made to 26a1c8e.

WAL-375

TICKET: WAL-375
mrdanish26 added a commit that referenced this pull request Apr 16, 2026
@mrdanish26
Merge pull request #8538 from BitGo/revert-WAL-375-8462
4584ba5 
Revert "Merge pull request #8462 from BitGo/WAL-375-mpcv2-sign-tx-req
@mrdanish26 mrdanish26 mentioned this pull request Apr 17, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pranavjain97 pranavjain97 pranavjain97 approved these changes

@zhongxishen zhongxishen zhongxishen approved these changes

@Marzooqa Marzooqa Awaiting requested review from Marzooqa Marzooqa is a code owner automatically assigned from BitGo/wallet-core-india

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@mrdanish26 @sachushaji @pranavjain97 @zhongxishen