From 42aed1601fcea8a89bcb4ad16e46d5381dcf39a4 Mon Sep 17 00:00:00 2001 From: adenueltech Date: Sat, 17 Oct 2026 21:55:34 +0100 Subject: [PATCH] docs(security): update init front-running status and residual risk (#104) The init front-running fix for access-lease (#43, review item 11) was recorded as SV-12 "Resolved", but the admin.require_auth() check and its test were dropped when PR #141 was merged, and subscription and vault-registry never had it. Mark SV-12 as open/regressed across all three contracts, correct the status line and access-control matrix, and add the init front-running residual risk with operational mitigations until the check is restored. Closes #104 Co-Authored-By: Claude Opus 5.5 (1M context) --- SECURITY_AUDIT.md | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/SECURITY_AUDIT.md b/SECURITY_AUDIT.md index 8040cc4..b6a7879 100644 --- a/SECURITY_AUDIT.md +++ b/SECURITY_AUDIT.md @@ -3,7 +3,9 @@ **Scope:** `vault-registry`, `access-lease`, `subscription` — initialisation, access control, expiry arithmetic, revocation, cancellation and the upgrade entrypoints. -**Status:** All High and Medium findings below are resolved in the current code. +**Status:** All High and Medium findings below are resolved in the current code +**except SV-12** (unauthenticated `init`), whose fix was lost in a later merge — +see [SV-12](#1-findings) and [Residual risk](#4-residual-risk). Each finding lists its severity, the issue, and how it is resolved. Every resolution is covered by a unit test in the contract's test module. @@ -25,7 +27,7 @@ resolution is covered by a unit test in the contract's test module. | SV-9 | subscription | Low | `deactivate_plan` and `cancel` wrote entries without bumping their TTL, so entries could be archived. | TTL bumped on every write. | Resolved | | SV-10 | all | Low | access-lease and subscription emitted no events, so there was no on-chain audit trail for grants, revocations, subscriptions or upgrades. | Events on every state change and on `upgrade` (see `docs/CONTRACTS.md`). | Resolved | | SV-11 | access-lease, subscription | Low | Instance storage (holding the admin) was only bumped at `init` and could expire. | Instance TTL bumped on every admin-gated call. | Resolved | -| SV-12 | access-lease | High | `init` did not require the admin's signature, so a front-runner could call `init` first after deployment and install themselves as admin, taking over every admin-only entrypoint. | `init` calls `admin.require_auth()` before writing any state, so only the intended admin can initialise the contract. Test: `init_requires_admin_auth`. | Resolved | +| SV-12 | access-lease, subscription, vault-registry | High | `init` does not require the admin's signature, so a front-runner can call `init` first after deployment and install themselves as admin, taking over every admin-only entrypoint (`upgrade` and `migrate` everywhere; `set_admin` and granting leases/subscriptions on access-lease and subscription). | Fixed for access-lease by #43 (`c7eda2e`: `admin.require_auth()` in `init`, test `init_requires_admin_auth`), but the check and its test were dropped when PR #141 was merged (`eee2a5d`). `subscription` and `vault-registry` never had it. Fix: call `admin.require_auth()` in every `init` before writing state and restore the test. Until then, mitigated operationally (see §4). | **Open (regressed)** | ## 2. Access-control matrix (verified by tests) @@ -34,7 +36,7 @@ resolution is covered by a unit test in the contract's test module. | `vault-registry::register` | `creator` | `mutations_require_creator_auth` | | `vault-registry::set_price` etc. | stored creator | `set_price_by_non_creator_is_rejected` | | `vault-registry::upgrade` | admin | `upgrade_rejects_non_admin` | -| `access-lease::init` | admin | `init_requires_admin_auth` | +| `access-lease::init`, `subscription::init`, `vault-registry::init` | **none** — see SV-12 | — (test removed; see SV-12) | | `access-lease::grant_lease` / `extend_lease` / `revoke_lease` | admin | `grant_by_non_admin_is_rejected` | | `subscription::subscribe` / `renew` | admin | `subscribe_by_non_admin_is_rejected` | | `subscription::deactivate_plan` | publisher | `deactivate_requires_publisher_auth` | @@ -52,6 +54,23 @@ resolution is covered by a unit test in the contract's test module. ## 4. Residual risk +- **Initialisation front-running (SV-12, open).** `init` on all three + contracts accepts any `admin` address without that address's signature, and + the first successful call wins (`AlreadyInitialised` blocks every later one). + Between deployment and `init`, anyone watching the network can call `init` + with their own address and take over `upgrade`, `migrate` and every other + admin-only entrypoint. Until `admin.require_auth()` is restored in + `init`: + - deploy and initialise in the same pipeline run with no gap, submitting + `init` from the deployer account immediately after the deploy confirms; + - immediately call `admin()` and confirm it returns the intended address; + - if it does not, abandon that contract ID and redeploy — the admin cannot + be recovered, because `set_admin` and `upgrade` are gated by the + (attacker's) admin. + + Once the fix is restored this item can be removed and SV-12 marked Resolved + again. + - **Admin key compromise.** The admin can upgrade code and grant arbitrary leases. Use a hardware-backed or multisig account on mainnet and rotate with `set_admin` if needed.