Skip to content

🟠 Admin governance is single-step and unguarded: one typo permanently bricks the factory #1012

Description

@Ejirowebfi

Area: Smart contract · lib.rs (transfer_admin, update_admin, upgrade)

Description

transfer_admin/update_admin immediately reassign state.admin to whatever address is passed. If
the admin fat-fingers an address (or transfers to a contract address that cannot sign), all
privileged operations — fee updates, pause/unpause, whitelist, upgrade, migrate — are lost
forever; there is no recovery. Two redundant functions performing the same mutation (one silent, one
with an event) doubles the attack/mistake surface and splits the audit trail. upgrade is similarly
instantaneous: a compromised or coerced admin key can atomically swap the executable for malicious
WASM with zero delay and zero on-chain warning, and nothing forces migrate to run afterwards, so an
upgrade requiring a schema change can leave state and code out of sync (Issue 2's migration depends on
this working).

Tasks

  • Implement two-step admin rotation: propose_admin(new_admin) stores a pending admin;
    accept_admin() requires new_admin.require_auth() to complete; cancel_admin_proposal() for
    the current admin. Remove the redundant duplicate function (keep one, emitting events for
    propose/accept/cancel).
  • Add an optional upgrade timelock: propose_upgrade(new_wasm_hash) + apply_upgrade() callable
    only after N ledgers, with an event on propose so watchers can react; document the emergency
    trade-off chosen.
  • Make upgrade/apply_upgrade automatically invoke the migration path (or hard-fail all
    entrypoints until migrate runs) so code/schema can never be observed out of sync.
  • Emit events for every governance action, including transfer_admin (currently silent).
  • Tests: pending-admin flow, cancel flow, auth failures, timelock boundaries, upgrade-then-migrate
    enforcement.
  • Update docs/contract-abi.md, docs/incident-response.md, and the mainnet checklist.

Acceptance criteria

  • Admin can only move to an address that has proven it can sign (accept step), proven by tests.
  • An upgrade cannot leave the contract serving new code against an old schema.
  • Every governance mutation emits an auditable event.

Issue 8 of 20 from the codebase audit tracked in ISSUES.md.

Metadata

Metadata

Assignees

No one assigned

    Labels

    GrantFox OSSIssue tracked in GrantFox OSSMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial Campaign | FWC26Campaign: Official Campaign | FWC26auditFrom the ISSUES.md codebase auditseverity: highBroken core feature or exploitable abuse vector

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions