Skip to content

fix(abstract-eth): verify EVM TSS bytes against intent before signing - #10121

Draft
dpkjnr wants to merge 3 commits into
masterfrom
CECHO-2460-evm-tss-fail-closed
Draft

dpkjnr wants to merge 3 commits into
masterfrom
CECHO-2460-evm-tss-fail-closed

Conversation

@dpkjnr

@dpkjnr dpkjnr commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

What

Makes EVM TSS signing fail closed: before the user key share signs, verifyTssTransaction decodes the transaction bytes and compares to / value / data with the user intent, on both signing paths (direct send = the caller's txParams; pending approval = the stored txRequest.intent).

This PR replaces the four-PR stack #9972 → #9973 → #9975 → #10114 with one change. The final tree is byte-identical to #10114's head; the content is the same, regrouped into three commits so no commit carries the pre-review verifier:

  1. fix(sdk-core): carries the stored intent, the caller's untouched txParams (callerTxParams) and the chain name into verifyTransaction; adds the client-side strictTssVerification / onVerificationWarning options and the TssVerificationWarning types; lets stored kebab-case DeFi intents past the missing-recipients guard.
  2. fix(abstract-eth): the fail-closed verifier (tssVerification.ts), with the fix(abstract-eth): verify EVM TSS payments against declared asset #10114 review fixes folded in:
    • The intent type picks a handler, and each handler accepts a fixed set of shapes: native send, ERC-20 / ERC-7984 transfer, exact dApp call, batcher batch / batchTransferFrom, tokenApproval to the batcher for MAX, fillNonce, consolidation, and the DeFi calldata checks. Anything else is a violation; contractCall / customTx throw.
    • When the caller declares a type or recipients, only its own params count. The stored intent is used only when the caller declared nothing.
    • The asset is resolved from local identity only, and the calldata shape must match it. Unresolvable or conflicting identity is a violation.
    • Removes EvmCoin.verifyLegacyTssTransaction, so kavaevm / prividiumeth run the parent verifier instead of returning true.
  3. test(sdk-coin-evm): acceptance suites on kavaevm, hbarevm and tbsc (batcher).

Why

TOB-BITGOEDMPC-2 / INFOSEC-671: verifyTssTransaction trusted the server-declared txParams.type. A compromised server could label approve(attacker, MAX) or any contract call as a payment, or redirect a payment on the pending-approval path, and the key share would sign it.

The stack could not land piece by piece: #9972 relaxes the DeFi recipient guard that #9973's handlers rely on, so master would sign stored DeFi intents unchecked between the two merges, and #10114 rewrote the core of #9973.

Case (default / strictTssVerification) master this PR
approve(attacker, MAX) under a native payment intent passes silently warns / throws
ERC-20 transfer on a different token contract than declared passes silently warns / throws
contractCall intent passes silently throws / throws
native redirect or inflated value on pending approval not checked throws / throws
legitimate native send, legitimate ERC-20 transfer pass pass / pass

Test plan

  • Each commit's tree tested locally (Node 24.22.1):
  • Prettier and ESLint clean on all changed files (0 errors).
  • BitGo SDK CI on this PR.
  • Release gate: prod backtest replaying ~563k EVM TSS payments from the last 90 days, expecting no unexplained rejection of legitimate payments (tracked on CECHO-2460).

Open items

  • defi-redeem has no handler. With this PR it reaches the verifier and only warns in default mode, where master rejected it at the recipients guard. Decide before release: drop the redeem entries from NO_RECIPIENT_TX_TYPES, or make that violation fatal.
  • strictTssVerification defaults to off, so the new checks log rather than block for default clients until a follow-up release flips it.
  • Adds an optional callerTxParams to VerifyTransactionOptions. Out-of-repo verifyTransaction implementations won't get the caller / stored split until they read it.

Supersedes #9972, #9973, #9975, #10114.

Ticket: CECHO-2460

🤖 Generated with Claude Code

dpkjnr and others added 3 commits November 3, 2026 20:32
The EVM TSS verifier cannot check a pending-approval transaction against
what the user approved, because the signing path only hands it the
resolveEffectiveTxParams output. A compromised server can then label
arbitrary calldata as a payment and the user key share signs it
(TOB-BITGOEDMPC-2 / INFOSEC-671).

The ecdsa and ecdsaMPCv2 signing paths now pass the stored
txRequest.intent, the caller's untouched txParams (callerTxParams) and
the chain name into verifyTransaction. The caller's params are passed
separately because once resolveEffectiveTxParams back-fills type and
recipients from the stored intent, txParams alone cannot tell what the
caller declared. The chain name also fixes token-name resolution from
amount.symbol, matching the EdDSA path.

BitGoAPI gains the client-side strictTssVerification and
onVerificationWarning options, set from local options only and never
from server data, plus the TssVerificationWarning reporting types.

The kebab-case DeFi spellings (defi-approve, defi-deposit,
defi-withdraw, defi-redeem) join NO_RECIPIENT_TX_TYPES, so stored DeFi
intents reach the verifier instead of failing the missing-recipients
guard. This only waives that guard: the abstract-eth verifier in the
next commit checks approve, deposit and withdraw calldata and flags
defi-redeem, which has no handler yet. The two must ship together.

Ticket: CECHO-2460
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
verifyTssTransaction trusted the server-declared txParams.type and only
decoded two calldata shapes under transfer. A compromised server could
label approve(attacker, MAX) or any contract call as a payment, or
redirect a payment on the pending-approval path, and the user key share
would sign it (TOB-BITGOEDMPC-2 / INFOSEC-671).

The verifier now decodes to/value/data and compares them with the
intent. The intent type picks a handler, and each handler accepts a
fixed set of shapes: native send, ERC-20 or ERC-7984 transfer, exact
dApp call, batcher batch/batchTransferFrom, tokenApproval to the
batcher for MAX, fillNonce, consolidation to the base address, and the
DeFi calldata checks. Anything else, including a type with no handler,
is a violation. contractCall and customTx throw, since EVM cannot
create them.

Exactly one source speaks for the user. When the caller declares a
type or recipients, only its own params count, and the stored intent is
used only when the caller declared nothing (pending approval). The
asset comes from local identity only: the recipient's tokenAddress or
tokenName, the top-level tokenName, and the coin instance. Identity
that cannot be resolved, or that names two assets, is a violation, and
the calldata shape must match the asset.

Checks that threw before keep throwing, now on the pending-approval
path too. New checks warn through console.warn and the optional
onVerificationWarning callback, and throw when the client sets
strictTssVerification. EvmCoin.verifyLegacyTssTransaction, which
returned true without decoding, is removed so kavaevm and prividiumeth
run the parent verifier. The sdk-coin-eth wrapper-approve tests move
off customTx, which now throws, and the README documents the contract.

Ticket: CECHO-2460
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The verifier changes what EVM TSS signing accepts, so the audit cases
need regression coverage that fails if a check quietly stops firing.

The sdk-coin-evm suite runs on kavaevm (legacy) and hbarevm (EIP-1559).
It covers mismatched destination, value and calldata, approve(attacker,
MAX) labelled as a payment, ERC-20 and ERC-7984 transfers,
tokenApproval, fillNonce, unhandled intent types, the pending-approval
path through resolveEffectiveTxParams with a stored intent (defi-*
intents included, with tampered amounts rejected), networks with no
configured batcher, and the warning payload. It also pins intent
isolation and asset binding: stored calldata cannot fill a caller
recipient, a server-named token cannot replace the caller's token, and
native and token shapes cannot be swapped.

The sdk-coin-bsc suite covers batcher flows on tbsc, which has a real
batcher: native batch and token batchTransferFrom happy paths and
mismatches, selector pinning, single-asset batches, and approve(batcher,
MAX) against approve(attacker, MAX).

Ticket: CECHO-2460
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

CECHO-2460

@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

⚠️ Compatibility checks are failing on Node.js 27-nightly (experimental, non-blocking). This does not block merge, but it signals an incompatibility to fix before this release becomes supported.

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.

1 participant