Repository navigation
fix(abstract-eth): fail-closed EVM TSS handlers for verifyTssTransaction - #9973
Draft
ralph-bitgo[bot] wants to merge 1 commit into
Draft
ralph-bitgo[bot] wants to merge 1 commit into
ralph-bitgo[bot] wants to merge 1 commit into
Conversation
What: make AbstractEthLikeNewCoins.verifyTssTransaction compare the decoded to/value/data against the user intent on every EVM TSS signing path. The intent type picks the handler (payment rules R1-R5, transferToken, tokenApproval, fillNonce, consolidate, DeFi); anything unmatched or unhandled is a violation instead of falling through to true. contractCall/customTx now throw on EVM. Stored kebab-case intent types (payment, defi-*) map to canonical verifier types and the stored intent supplies defiParams / tokenName fields. EvmCoin.verifyLegacyTssTransaction is removed so non-EIP-1559 chains (kavaevm, prividiumeth) run the parent verifier. Why: TOB-BITGOEDMPC-2 / INFOSEC-671 - a compromised server could label arbitrary calldata (e.g. approve(attacker, MAX)) as a transfer and the client key share would sign it unchallenged. Checks that threw before keep throwing; every new check warns (console.warn plus an optional onVerificationWarning callback) and throws under the client-side strictTssVerification option, so the checks roll out without rejecting legitimate requests. Ticket: CECHO-2460 Session-Id: 22db0dc1-cf47-40e2-b717-1bb226a6304c Task-Id: a8a26387-a332-4be1-aab5-f39deddfc502
Contributor
This was referenced Oct 14, 2026
dpkjnr
added this pull request to stack #9979
October 15, 2026 12:38
Contributor
|
|
This branch has not been deployed
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.
What
Makes
AbstractEthLikeNewCoins.verifyTssTransactionfail-closed: the user's key share must not sign EVM TSS bytes unless the decodedto/value/datamatch the intent, on both signing paths (direct send = callertxParams; pending approval = storedtxRequest.intent).payment → transfer,defi-approve → defiApprove, …); caller-suppliedtxParams.typestill wins. Handlers accept a closed set of decoded shapes; anything unmatched or unhandled is a violation.transfer/payment, rules R1–R5): 1 recipient withdata(WalletConnect/dApp) →to/value/dataexactly equal to the intent; 1 native recipient → emptydata, matchingto/value; 1 token recipient →to= token contract (statics ortokenData.tokenContractAddress),value= 0, decodedtransfer(address,uint256)(or ERC-7984confidentialTransferrecipient-only) matching; >1 recipients →to= the networkbatcherContractAddress,batch(address[],uint256[])arrays 1:1 withvalue= sum, orbatchTransferFrom(address,address[],uint256[])with the intent token andvalue= 0; anything else (e.g.approve(attacker, MAX)disguised as a payment) is a violation.tokenApproval:to= token contract,approve(spender = batcher, MAX),value= 0.transferToken: same as the token-recipient rule, exactly one recipient.fillNonce: 1-wei self-send to the wallet base address.consolidate/consolidateId: the base-address check now runs automatically, not only underverification.consolidationToBaseAddress.defiApprove/defiDeposit/defiWithdraw: existing calldata checks, now reachable on the pending-approval path withdefiParamsreconstructed from storedamount/shareTokenAmount.contractCall/customTx: throwunsupported intent type for EVM TSS— neither can be created on EVM and there is no intent shape to verify.acceleration,bridgeFunds,enableToken/enabletoken/disableToken/disabletoken,wrapNative/wrap-native,unwrapNative/unwrap-native,wrap,unwrap,finalizeUnwrap,defiRedeem/defi-redeem, staking (delegate,undelegate,switchValidator,stake,unstake,stakeWithCallData,unstakeWithCallData,transferStake,increaseStake,goUnstake,claim,stakeClaimRewards),createAccount,import,importtoc,pledge, and any other type without a handler.to/value, ERC-20 inner recipient/amount, DeFi calldata, wrapApprove, consolidation underverification.consolidationToBaseAddress, missing params) — now also on the pending-approval path. Every new check warns (console.warn+ optionalonVerificationWarningcallback carrying decoded vs expectedto/value/data); the client-side opt-instrictTssVerification: trueturns warnings into hard failures. Strict-by-default is a separate follow-up release.EvmCoin.verifyLegacyTssTransactionso non-EIP-1559 TSS chains (kavaevm,tkavaevm,prividiumeth,tprividiumeth) run the parent verifier instead of returningtruewithout decoding.Why
TOB-BITGOEDMPC-2 / INFOSEC-671:
verifyTssTransactiontrusted the server-declaredtxParams.typeand only validated two calldata shapes undertransfer. A compromised server could labelapprove(attacker, MAX)(or any contract call) as a payment, or redirect/inflate a payment on the pending-approval path wheretypeis the storedintentType(payment), and the key share would sign it unchallenged.Test plan
bitgofull unit suite.customTxbehaviour (unsupported types reject outright; the wrapper-approve guard's relabel cases re-based onto still-reachable types).Stack
This PR is part 2 of 3; review and merge in order:
master)CECHO-2460-evm-tss-fail-closed-handlers-pt1) ← you are hereCECHO-2460-evm-tss-fail-closed-handlers-pt2)Ticket: CECHO-2460