Skip to content

fix(abstract-eth): fail-closed EVM TSS handlers for verifyTssTransaction - #9973

Draft
ralph-bitgo[bot] wants to merge 1 commit into
CECHO-2460-evm-tss-fail-closed-handlers-pt1from
CECHO-2460-evm-tss-fail-closed-handlers-pt2
Draft

ralph-bitgo[bot] wants to merge 1 commit into
CECHO-2460-evm-tss-fail-closed-handlers-pt1from
CECHO-2460-evm-tss-fail-closed-handlers-pt2

Conversation

@ralph-bitgo

@ralph-bitgo ralph-bitgo Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

What

Makes AbstractEthLikeNewCoins.verifyTssTransaction fail-closed: the user's key share must not sign EVM TSS bytes unless the decoded to / value / data match the intent, on both signing paths (direct send = caller txParams; pending approval = stored txRequest.intent).

  • Intent type picks the handler. Stored kebab-case types map to canonical verifier types (payment → transfer, defi-approve → defiApprove, …); caller-supplied txParams.type still wins. Handlers accept a closed set of decoded shapes; anything unmatched or unhandled is a violation.
    • Payment (transfer / payment, rules R1–R5): 1 recipient with data (WalletConnect/dApp) → to/value/data exactly equal to the intent; 1 native recipient → empty data, matching to/value; 1 token recipient → to = token contract (statics or tokenData.tokenContractAddress), value = 0, decoded transfer(address,uint256) (or ERC-7984 confidentialTransfer recipient-only) matching; >1 recipients → to = the network batcherContractAddress, batch(address[],uint256[]) arrays 1:1 with value = sum, or batchTransferFrom(address,address[],uint256[]) with the intent token and value = 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 under verification.consolidationToBaseAddress.
    • defiApprove / defiDeposit / defiWithdraw: existing calldata checks, now reachable on the pending-approval path with defiParams reconstructed from stored amount / shareTokenAmount.
    • contractCall / customTx: throw unsupported intent type for EVM TSS — neither can be created on EVM and there is no intent shape to verify.
  • Unhandled types warn instead of silently passing: 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.
  • Rollout (per settled design): checks that threw before keep throwing (native to/value, ERC-20 inner recipient/amount, DeFi calldata, wrapApprove, consolidation under verification.consolidationToBaseAddress, missing params) — now also on the pending-approval path. Every new check warns (console.warn + optional onVerificationWarning callback carrying decoded vs expected to/value/data); the client-side opt-in strictTssVerification: true turns warnings into hard failures. Strict-by-default is a separate follow-up release.
  • Removes EvmCoin.verifyLegacyTssTransaction so non-EIP-1559 TSS chains (kavaevm, tkavaevm, prividiumeth, tprividiumeth) run the parent verifier instead of returning true without decoding.

Why

TOB-BITGOEDMPC-2 / INFOSEC-671: verifyTssTransaction trusted the server-declared txParams.type and only validated two calldata shapes under transfer. A compromised server could label approve(attacker, MAX) (or any contract call) as a payment, or redirect/inflate a payment on the pending-approval path where type is the stored intentType (payment), and the key share would sign it unchallenged.

Test plan

  • Existing suites stay green with the strengthened contract: sdk-coin-eth 460, sdk-coin-bsc 41, sdk-coin-xdc 65, sdk-coin-evm 40, sdk-core 966, bitgo full unit suite.
  • sdk-coin-eth contract tests updated for the new customTx behaviour (unsupported types reject outright; the wrapper-approve guard's relabel cases re-based onto still-reachable types).
  • Acceptance coverage for the new rules lands in part 3.

Stack

This PR is part 2 of 3; review and merge in order:

  1. fix(sdk-core): carry TSS intent and strict policy to verifyTransaction #9972 — sdk-core intent + strict policy plumbing (base: master)
  2. fix(abstract-eth): fail-closed EVM TSS handlers for verifyTssTransaction #9973 — abstract-eth fail-closed verifier (base: CECHO-2460-evm-tss-fail-closed-handlers-pt1) ← you are here
  3. test(sdk-coin-evm): cover fail-closed EVM TSS verification paths #9975 — verification test suites (base: CECHO-2460-evm-tss-fail-closed-handlers-pt2)

Ticket: CECHO-2460

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
@linear-code

linear-code Bot commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

CECHO-2460

@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Unit tests are failing on Node 26.x (Current release line, non-blocking). This is not an LTS version yet, so it does not block merge, but it signals an incompatibility to fix before Node 26.x becomes LTS.

View run

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants