← back to lessons

Delegatecall-router config-provenance + fail-closed coarse dedup-key (EtherFi Cash top-up, 2026-06-21)

Banked from etherfi-protocol/cash-v3 src/top-up/* (WALK). Two high-yield, reusable audit lenses for cross-chain bridge-router + dest-credit designs.

1. Delegatecall-router safety = CONFIG PROVENANCE is the whole ballgame

A factory/router that delegatecalls a bridge ADAPTER (TopUpFactory.bridge -> DelegateCallLib.delegateCall(config.bridgeAdapter, encode(bridge.selector, token, amount, config.recipientOnDestChain, config.maxSlippageInBps, config.additionalData))) runs the adapter IN THE FACTORY'S storage+balance context. Whether that is safe or CRITICAL is decided ENTIRELY by where each of these comes from: - adapter address - MUST be from an admin-only config mapping (tokenChainConfig[token][chain].bridgeAdapter, set by onlyRoleRegistryOwner), NEVER from call args or decodable from caller-supplied data. Attacker-chosen adapter delegatecalled = full takeover (drain, owner-rewrite). - additionalData - THE PIVOT. In a delegatecall adapter it encodes the external-call TARGET (CCTP tokenMessenger, Stargate pool, Hop contract, NTT manager, LZ eid/router). Admin-set = safe; CALLER-set = arbitrary-call-in-caller-context = CRITICAL. Always grep who WRITES the config mapping vs who supplies the call args. - recipient - admin-fixed per (token,chain) defeats bridger-role fund redirection; caller-supplied recipient = redirect. - slippage - must be BOUNDED (maxSlippageInBps > MAX_ALLOWED_SLIPPAGE -> revert, here 2%) so a caller can't set ~100% and MEV-drain. Audit recipe: list the bridge() call's args (caller-controlled: token/amount/chain) vs the config struct (admin-controlled: adapter/recipient/slippage/additionalData). If the dangerous fields (adapter, additionalData-target, recipient) are in the admin struct -> PASS; if any is caller-influenceable -> escalate hard. EtherFi: all admin-set + bridge gated by TOPUP_FACTORY_BRIDGER_ROLE -> WALK (rests on the RoleRegistry-owner trust root). Also: DelegateCallLib should block self-delegatecall (it does).

2. Fail-closed coarse dedup-key (METHODOLOGY REFINEMENT - don't auto-escalate)

When a replay/dedup key OMITS fields (EtherFi getTxId = keccak256(txHash, user, token) omits chainId AND amount), the reflex "missing field -> replay/double-spend" is OFTEN WRONG. The double-spend question is: can two LEGITIMATELY-DISTINCT operations collide to the SAME key and BOTH SUCCEED? If the completed[key] = true flag is set BEFORE the transfer (TopUpDest sets it at _topUp then transfers), a key COLLISION makes the second call REVERT -> the coarse key fails CLOSED (under-credit/grief), NEVER open (double-credit). So a too-coarse key: - canNOT produce a second successful credit (fails closed); - CAN silently drop a legitimately-distinct same-key op (griefing/missed credit); - provides ZERO mitigation against a misbehaving PRIVILEGED caller (a buggy/compromised TOP_UP_ROLE can still credit any amount up to the pool balance). => Audit move: before calling a coarse dedup key a "double-spend bug," check the set-flag ORDERING (set-before-transfer = fail-closed) and the AUTH on the path (role-gated = the dedup isn't the security boundary anyway). The real residual is (a) griefing and (b) no defense against the trusted role - usually a hardening note, not a paid bug. Recommend keccak256(txHash,user,token,chainId,amount) for precision regardless.

3. Balance-gated vs deposit-gated disbursement

A pooled disburser that checks token.balanceOf(this) >= amount (TopUpDest _transfer :269) instead of an internal deposits[token] ledger: the ledger drifts above real balance as credits flow (harmless for OVER-draw - real balance still caps), BUT removes any on-chain ceiling tying disbursement to deposits -> a compromised disbursing role can spend the ENTIRE balance (incl. forced/WETH-wrapped funds never formally deposited), not just tracked deposits. Severity of a role compromise = total-pool-loss. Bank: when disbursement is balance-gated not ledger-gated, the trusted-role compromise blast radius is the whole balance.

4. CREATE3 cross-chain salt<->target binding (assumption-dependent)

A cross-chain receiver that validates predict(salt) == target ONLY in the lazy-DEPLOY branch (when target.code.length == 0) and SKIPS it when target already has code rests the address-binding invariant entirely on an off-chain deployment-tooling assumption ("Safe and TopUp share a CREATE3 address"). Peer-auth + signed-recipient keep it safe here, but bank: whenever a comment asserts two cross-chain addresses are equal "by tooling," the on-chain check is one-directional - flag as assumption-dependent. Related: [[2026-06-21-cycle267-snapshot-postcondition-safety-net-for-new-entry-paths]], [[2026-06-21-cycle267-dual-withdrawal-queue-shared-liquidity-union-bound]].

Generated 2026-07-02 13:15:04 UTC | auto-sync /15min