🌳 Advanced ⏱️ 50 min

Access Control Vulnerabilities

Access control is the most frequently exploited category of smart contract vulnerability in terms of total dollar value lost. The Ronin Bridge lost $625M because a validator key had too much authority. The Parity wallet froze $150M because a public initialization function was never called during deployment. These bugs are often simple to spot once you know what to look for — but catastrophic when missed in production.

🚨 Critical

Access control bugs account for the largest share of funds stolen from DeFi protocols. Every function that modifies contract state, upgrades logic, or moves funds MUST have an explicit, correct access check. "Public" is not the right default for sensitive functions.

The Core Skill: Reading Intent Before Reading Code

Most auditors fail access control reviews not because they don't know what modifiers are, but because they evaluate code in isolation without first establishing what the protocol intended. A function with onlyOwner is not automatically correct — maybe it should be callable by anyone. A function with no modifier is not automatically wrong — maybe public access is the design. The first step is always to build an expected access model, then compare it to what the code actually enforces.

🧠 The Mental Model

Before reading a single modifier, answer: Who are the actors in this system? A typical DeFi protocol has: (1) Users — deposit, withdraw, trade. (2) Operators/Keepers — trigger liquidations, harvest yields, rebalance. (3) Admin/Governance — change parameters, upgrade contracts, pause. (4) The Protocol itself — internal callbacks, cross-contract calls. Every function belongs to exactly one actor group. If the code says otherwise, that's your finding.

The 4-State Access Control Taxonomy

Every function in a codebase falls into one of four states. Only the first is correct. The other three are all bugs — but they have very different impacts and fixes:

State	What It Means	Example	Severity
✅ Correct	Restriction matches design intent	`setFee() onlyGovernance` — fees should be governance-controlled	N/A
❌ Under-restricted	Function is open but should be gated	`mint() external` with no modifier — anyone can print tokens	Critical / High
❌ Over-restricted	Function is gated but should be open (or less restricted)	`triggerLiquidation() onlyOwner` — liquidators can't act without admin approval, creating bad debt	Medium / High
❌ Wrong-role	Gated to the wrong actor	`emergencyPause() onlyOperator` when only governance should pause — or vice versa	Medium / High

📋 Step 1 — Build the Role Map Before Reading Code

// BEFORE opening any .sol file, read the README/docs and write this out:

// ACTOR MAP — Example: Lending Protocol
// ─────────────────────────────────────────────────────────────
// Actor: USER (any EOA or contract)
//   SHOULD be able to: deposit(), withdraw(), borrow(), repay()
//   SHOULD NOT be able to: setLTV(), pause(), upgrade(), grantRole()

// Actor: LIQUIDATOR (any address willing to repay bad debt)
//   SHOULD be able to: liquidate() — permissionless, incentivized
//   SHOULD NOT be able to: change liquidation bonus, pause other users

// Actor: OPERATOR (protocol team multisig)
//   SHOULD be able to: pause(), setInterestRate(), add collateral types
//   SHOULD NOT be able to: upgrade implementation, drain funds

// Actor: GOVERNANCE (timelock + token holders)
//   SHOULD be able to: upgrade(), setFee(), grantRole(OPERATOR), emergencyDrain()
//   Time-delayed — not instant

// Actor: PROTOCOL (internal — called by other contracts in the system)
//   SHOULD be able to: internal accounting callbacks
//   SHOULD NOT be triggered by external callers

// Now read the code and check every function against this map.
// Any mismatch = potential finding.

Under-Restriction: Missing Guard (Classic Bug)

The most dangerous case: a function that should be restricted is fully public. Any address in the world can call it.

⚠️ Under-Restricted — liquidate() Should Be Open, mint() Should Not

contract LendingProtocol {
    // ✅ CORRECT: liquidate() is intentionally permissionless
    // Any address can liquidate an unhealthy position — this is by design
    // Restricting it would prevent liquidations and create bad debt
    function liquidate(address borrower) external {
        require(isUndercollateralized(borrower), "Position healthy");
        // Liquidator repays debt, receives collateral at discount
    }

    // ❌ WRONG: mint() should require MINTER_ROLE but is open
    // When reading the docs: "Only whitelisted minters can create new tokens"
    // When reading the code: no modifier — INTENT MISMATCH = FINDING
    function mint(address to, uint256 amount) external {
        balances[to] += amount;  // Anyone can inflate supply
    }

    // ❌ WRONG: harvestYield() looks like it should be restricted
    // But the docs say "anyone can trigger harvests — it's permissionless"
    // Reading code: onlyOwner — OVER-RESTRICTION = FINDING
    function harvestYield() external onlyOwner {
        // Users can't trigger their own yield harvest without admin help
        // This is a centralization/DoS finding — admin can grief by not harvesting
    }
}

Over-Restriction: Role-Gating That Creates DoS or Centralization Risk

The opposite mistake: a function IS gated, but shouldn't be — or is gated to a role that is too narrow. This creates DoS vectors (functionality locked until admin acts) and centralization risk (one address has too much power over user funds).

⚠️ Over-Restricted — Liquidation Blocked by Admin Gate

contract OverRestrictedLending {
    address public admin;

    // ❌ CRITICAL over-restriction: liquidation requires admin approval
    // If admin is offline, slow, or malicious — unhealthy positions accumulate
    // Protocol accrues bad debt that cannot be recovered
    function liquidate(address borrower) external {
        require(msg.sender == admin, "Only admin can liquidate");
        // ❌ Liquidators have no ability to act — admin is a bottleneck
    }

    // ❌ Medium over-restriction: only admin can trigger reward distribution
    // Users must wait for admin to call this — centralized dependency
    function distributeRewards() external {
        require(msg.sender == admin, "Not admin");
        // This should be permissionless — a keeper or anyone should trigger it
    }

    // ❌ Medium centralization: admin can pause withdrawals at will
    // Users cannot exit — funds effectively locked by admin decision
    // Even if "intended", this is typically a Medium finding in audit reports
    function pause() external {
        require(msg.sender == admin);
        paused = true;
        // ← No timelock, no governance vote, no delay. Instant fund lock.
    }

    // ✅ FIXED: liquidate is permissionless with a health check
    function liquidateSafe(address borrower) external {
        require(healthFactor(borrower) < 1e18, "Position healthy");
        // Incentive structure ensures liquidators self-select — no admin needed
    }
}

Wrong-Role: Gated to the Right Concept but Wrong Actor

A function has access control, and some form of restriction makes sense — but the specific role or address being checked is wrong. These are subtle and require reading the full role architecture to spot.

⚠️ Wrong-Role — Correct Pattern, Wrong Actor Checked

contract YieldVault {
    bytes32 public constant STRATEGIST_ROLE = keccak256("STRATEGIST_ROLE");
    bytes32 public constant GUARDIAN_ROLE  = keccak256("GUARDIAN_ROLE");
    bytes32 public constant GOVERNANCE_ROLE = keccak256("GOVERNANCE_ROLE");

    // ❌ Wrong role: emergencyExit() is restricted to STRATEGIST
    // Protocol intent (from docs): GUARDIAN should trigger emergency exits
    // if a strategist is compromised — but now strategist controls the escape hatch
    // A compromised strategist can drain AND block emergency response
    function emergencyExit() external onlyRole(STRATEGIST_ROLE) {
        // Should be onlyRole(GUARDIAN_ROLE)
    }

    // ❌ Wrong role: setStrategy() requires GOVERNANCE but should require STRATEGIST
    // Governance changes take 48h (timelock) — during a market emergency,
    // the protocol can't rotate strategies fast enough
    function setStrategy(address newStrategy) external onlyRole(GOVERNANCE_ROLE) {
        // Routine strategy rotation should be STRATEGIST, not slow governance
    }

    // ✅ CORRECT role mapping:
    // GOVERNANCE → setFee(), upgrade(), add new strategy types
    // STRATEGIST  → setStrategy(), adjustAllocation()
    // GUARDIAN    → emergencyExit(), pause()
    // USER        → deposit(), withdraw(), claimRewards()
}

✏️ Try It — Access Control Audit Walkthrough

// STEP-BY-STEP: How to audit access control on a new codebase

// STEP 1: Read docs, extract actor list
// → "Keepers trigger harvests. Governance sets fees. Users deposit/withdraw."

// STEP 2: List every external/public function + expected caller
// deposit()         → USER        → open ✓
// withdraw()        → USER        → open ✓
// harvest()         → KEEPER      → open (permissionless) ✓
// setFee()          → GOVERNANCE  → timelocked ✓
// pause()           → GUARDIAN    → fast, no delay ✓
// upgrade()         → GOVERNANCE  → timelocked ✓

// STEP 3: Read actual modifiers and compare
// deposit()   → no modifier        → ✅ matches: open
// harvest()   → onlyOwner          → ❌ MISMATCH: should be permissionless
// setFee()    → onlyOwner          → ❌ MISMATCH: should be governance (timelocked)
// pause()     → onlyOwner          → ❌ MISMATCH: should be guardian role
// upgrade()   → onlyOwner          → ❌ MISMATCH: no timelock, instant upgrade

// STEP 4: Write findings
// Finding 1 (Medium): harvest() restricted to owner — centralization DoS risk
// Finding 2 (High):   setFee() has no timelock — admin can frontrun users
// Finding 3 (Medium): no role separation — single owner controls all functions
// Finding 4 (High):   upgrade() has no timelock — instant rug vector

💡 The Auditor's Rule of Thumb

When you see onlyOwner on a function, ask three questions in order: (1) Should this be restricted at all? (liquidate, harvest, and claim functions usually shouldn't be). (2) Is this the right role? (single owner vs role-based vs timelock-gated governance). (3) Is the role itself secure? (is the owner a multisig? is there a timelock?). A function can pass question 3 and still be a finding if it fails question 1 or 2.

Type 1: Missing Access Control — Public Admin Functions

The most basic form: a sensitive function is marked public or external with no modifier. Any address in the world can call it.

⚠️ Vulnerable — Anyone Can Become Owner

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

contract VulnerableToken {
    address public owner;
    mapping(address => uint256) public balances;

    constructor() {
        owner = msg.sender;
    }

    // ❌ CRITICAL: No access control — anyone can call this
    function setOwner(address newOwner) external {
        owner = newOwner;  // Attacker calls setOwner(attackerAddress)
    }

    // ❌ No modifier — anyone who becomes "owner" can drain all funds
    function drainFunds(address to) external {
        require(msg.sender == owner, "Not owner");
        (bool ok,) = to.call{value: address(this).balance}("");
        require(ok);
    }
}

// ✅ FIXED: Proper onlyOwner modifier
contract SecureToken {
    address public owner;

    modifier onlyOwner() {
        require(msg.sender == owner, "Not owner");
        _;
    }

    constructor() {
        owner = msg.sender;
    }

    // ✅ Protected — only current owner can call this
    function transferOwnership(address newOwner) external onlyOwner {
        require(newOwner != address(0), "Zero address");
        owner = newOwner;
    }
}

Type 2: tx.origin Authentication Bypass

tx.origin is the original EOA (externally owned account) that initiated the transaction. Using it for authentication allows a phishing attack: trick the real owner into calling a malicious contract, which then calls your protected function. Since tx.origin is the original owner's address, the check passes even though the intermediary contract is the direct caller.

⚠️ tx.origin Phishing Attack

// Vulnerable contract using tx.origin
contract VulnerableWallet {
    address public owner;

    constructor() { owner = msg.sender; }

    function transfer(address payable dest, uint256 amount) external {
        // ❌ tx.origin = the human who signed the transaction
        // This is NOT necessarily msg.sender when called through a contract
        require(tx.origin == owner, "Not owner");
        dest.transfer(amount);
    }
}

// Malicious contract — owner is tricked into calling this
// e.g., "Claim your free NFT!" — actually calls attack()
contract PhishingContract {
    VulnerableWallet public wallet;
    address payable public attacker;

    constructor(address _wallet, address payable _attacker) {
        wallet = VulnerableWallet(_wallet);
        attacker = _attacker;
    }

    // Owner calls this thinking it's a legitimate airdrop claim
    function claimFreeNFT() external {
        // During this call: tx.origin = owner's EOA, msg.sender = this contract
        // The wallet's tx.origin check passes! Funds drained to attacker.
        wallet.transfer(attacker, address(wallet).balance);
    }
}

// ✅ FIXED: Use msg.sender instead of tx.origin
function transfer(address payable dest, uint256 amount) external {
    require(msg.sender == owner, "Not owner");  // ✅ Direct caller check
    dest.transfer(amount);
}

⚠️ Warning

tx.origin has one legitimate use case: checking that the caller is a human EOA and not a contract (e.g., some anti-bot protections). However, it should NEVER be used to authorize privileged operations because of this phishing vector.

Type 3: Incorrect Modifier — Guard That Does Not Guard

A modifier can appear correct but fail to actually restrict access if it contains a logic error, is missing the _ placeholder, or uses the wrong comparison.

⚠️ Modifier That Fails to Protect

contract BrokenModifiers {
    address public owner;

    // ❌ Bug 1: Missing underscore — function body NEVER executes
    // Everyone is blocked, including the owner
    modifier onlyOwnerBroken1() {
        require(msg.sender == owner, "Not owner");
        // ← Missing _; — function body is never reached
    }

    // ❌ Bug 2: Wrong operator — allows everyone EXCEPT the owner
    modifier onlyOwnerBroken2() {
        require(msg.sender != owner, "Not owner");  // != should be ==
        _;
    }

    // ❌ Bug 3: Modifier exists but isn't applied to the function
    modifier onlyOwner() {
        require(msg.sender == owner);
        _;
    }
    // The modifier is defined but not used here:
    function adminAction() external { /* ❌ No modifier applied */ }

    // ❌ Bug 4: Empty require — always passes
    modifier onlyOwnerBroken4() {
        require(true);  // Hardcoded true — anyone passes
        _;
    }
}

Type 4: Unprotected Initializer — The Parity Hack

Proxy-based contracts cannot use constructors to set initial state — they use initialize() functions instead. If this function has no access control and is called during deployment but left callable afterward, anyone can reinitialize the contract and take ownership.

⚠️ Unprotected Initializer (Parity Pattern)

// This is similar to what brought down Parity's multisig library
contract WalletLibrary {
    address public owner;
    bool private initialized;

    // ❌ CRITICAL: Anyone can call this after deployment
    // The Parity library was deployed but initWallet() was never called
    // An attacker called it, became owner, then called kill()
    function initWallet(address[] memory _owners) public {
        owner = msg.sender;  // No check — anyone can become owner
        initialized = true;
    }

    function kill(address payable _to) external {
        require(msg.sender == owner);
        selfdestruct(_to);  // Attacker destroyed the shared library
    }
}

// ✅ FIXED: OpenZeppelin Initializable pattern
import "@openzeppelin/contracts/proxy/utils/Initializable.sol";

contract SecureWalletImplementation is Initializable {
    address public owner;

    // ✅ initializer modifier ensures this runs exactly once
    function initialize(address _owner) external initializer {
        owner = _owner;
    }

    // ✅ Disable initializers in implementation constructor
    // Prevents direct initialization of the implementation contract
    constructor() {
        _disableInitializers();
    }
}

Type 5: Privilege Escalation

A function grants roles without verifying the caller already has authority to grant them. This allows any address to self-promote to admin or mint new roles without authorization.

⚠️ Privilege Escalation — Unguarded Role Grant

contract VulnerableRoles {
    mapping(address => bool) public isAdmin;
    mapping(address => bool) public isMinter;

    constructor() { isAdmin[msg.sender] = true; }

    // ❌ Anyone can call this and grant themselves minter role
    function grantMinter(address account) external {
        isMinter[account] = true;  // No check that msg.sender is admin
    }

    // ✅ FIXED: Require caller to have the admin role
    function grantMinterFixed(address account) external {
        require(isAdmin[msg.sender], "Not admin");
        isMinter[account] = true;
    }
}

Type 6: Owner Renouncement — Permanent Lock

Calling renounceOwnership() sets the owner to the zero address. If any admin functions are protected by onlyOwner, they become permanently uncallable. Protocols that rely on admin functions for upgrades, emergency stops, or parameter adjustments are bricked.

⚠️ Renouncement Risk — Permanently Locked

import "@openzeppelin/contracts/access/Ownable.sol";

contract UpgradableProtocol is Ownable {
    address public implementation;

    function upgrade(address newImpl) external onlyOwner {
        implementation = newImpl;
    }

    function emergencyPause() external onlyOwner { /* ... */ }

    // ❌ After calling renounceOwnership(), upgrade() and emergencyPause()
    // are PERMANENTLY inaccessible — no one can call them ever again
    // Inherited from Ownable — caller must understand this risk
}

// ✅ Better: Override renounceOwnership to prevent accidental use
function renounceOwnership() public virtual override {
    revert("Renouncement disabled");
}

Access Control Patterns Comparison

Pattern	Use Case	Strengths	Weaknesses
Ownable (single owner)	Simple contracts, tokens	Simple, low gas	Single point of failure, renouncement risk
Ownable2Step	Production protocols	Prevents accidental ownership transfer	Still single owner
AccessControl (OZ)	Multi-role protocols	Granular roles, role admin system	More complex, higher gas
Custom modifier	Specific conditions	Flexible	Easy to write incorrectly
Gnosis Safe multisig	High-value admin ops	No single point of failure	Slow, requires coordination
Timelock + multisig	Protocol governance	Community can detect and react to changes	Delay can slow emergency response

Constructor vs Initialize: Why Proxies Are Dangerous

Proxy contracts separate storage (proxy) from logic (implementation). Because the proxy uses delegatecall to run the implementation's code in the proxy's storage context, the implementation's constructor runs in the implementation's own storage — not the proxy's. This means the constructor cannot initialize the proxy's storage. An initialize() function must be called on the proxy separately, creating a window where the proxy is uninitialized and potentially exploitable.

📋 Why Constructor Cannot Initialize Proxy State

// DEPLOYMENT FLOW FOR UPGRADEABLE CONTRACTS:
// 1. Deploy Implementation contract → constructor() runs → sets Implementation's storage
// 2. Deploy Proxy contract → points to Implementation
// 3. Call proxy.initialize() → runs Implementation's initialize() via delegatecall
//    → sets Proxy's storage (this is where user data lives)

// ❌ If step 3 is skipped or can be called again, attacker takes over

// Audit checklist for upgradeable contracts:
// □ Is initialize() protected by the initializer modifier?
// □ Does the implementation constructor call _disableInitializers()?
// □ Is there a reinitializer guard for V2/V3 upgrades?
// □ Is the proxy admin address controlled by a multisig, not an EOA?

contract SafeImplementation is Initializable, OwnableUpgradeable {
    uint256 public criticalValue;

    // ✅ Prevents anyone from calling initialize() on the implementation directly
    constructor() {
        _disableInitializers();
    }

    // ✅ Only callable once, on the proxy, by the deployer via the deployment script
    function initialize(address _owner, uint256 _criticalValue)
        external initializer {
        __Ownable_init(_owner);
        criticalValue = _criticalValue;
    }
}

Time-Lock Bypass: Frontrunning Governance

Governance timelocks give the community time to react to malicious proposals. But if the timelock delay is too short, or if certain functions bypass the timelock, attackers can push through changes before the community can respond.

⚠️ Timelock Bypass Patterns

contract TimelockController {
    uint256 public constant MINIMUM_DELAY = 2 days;
    mapping(bytes32 => uint256) public operationTimestamp;

    // ❌ Pattern 1: Delay too short — 2 minutes not 2 days
    uint256 public delay = 2 minutes; // Community has no time to react

    // ❌ Pattern 2: Admin can bypass timelock for "emergency" changes
    function emergencyExecute(bytes calldata data) external onlyAdmin {
        // Executes immediately without timelock — admin becomes single point of failure
        (bool ok,) = address(this).call(data);
        require(ok);
    }

    // ❌ Pattern 3: Timelock can be cancelled by the proposer
    // Attacker: queue malicious op → community reacts → cancel → try again
    // With enough proposals, can tire out community defenders

    // ✅ BEST PRACTICE:
    // - Minimum 48-72 hour delay for critical operations
    // - Cancellation requires higher threshold than execution
    // - No bypass functions — emergencies handled by circuit breakers, not admin override
    // - Timelock admin should be a multisig, not an EOA
}

Detection Checklist: Auditing Access Control

💡 Audit Checklist

For every function in the codebase, ask: (1) Who is allowed to call this? (2) Is there a modifier or require that enforces that? (3) Is the modifier implemented correctly? (4) Can the access check be bypassed through a proxy, delegatecall, or another function? (5) Are all admin key holders using multisig wallets?

🔍 Slither Detection Commands

# Run all access control detectors
slither . --detect unprotected-upgrade,suicidal,arbitrary-send,controlled-delegatecall

# Check for missing modifier on functions that match admin patterns
slither . --detect missing-zero-check,tx-origin

# List all functions with no access control (manual review candidates)
slither . --print function-summary | grep "external"

# Generate a function visibility report
slither . --print contract-summary

Real-World Exploits

Protocol	Year	Loss	Bug Type	What Happened
Parity Wallet	2017	$150M frozen	Unprotected initializer	Library's initWallet() was public; attacker called it, became owner, called kill()
Ronin Bridge	2022	$625M	Over-broad validator access	Sky Mavis controlled 5 of 9 validator keys; attacker compromised Sky Mavis private keys
Nomad Bridge	2022	$190M	Missing validation	A zero-value root was trusted; anyone could replay messages with arbitrary amounts
Euler Finance	2023	$197M	Missing access check	donateToReserves() had no health check — combined with flash loan to bypass solvency
Vulcan Forged	2021	$140M	Centralized key	Single private key controlled all wallets; key was compromised

Foundry Test: Access Control Enforcement

🧪 Foundry Test Template — Access Control

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import "forge-std/Test.sol";
import "../src/SecureToken.sol";

contract AccessControlTest is Test {
    SecureToken public token;
    address owner = makeAddr("owner");
    address attacker = makeAddr("attacker");

    function setUp() public {
        vm.prank(owner);
        token = new SecureToken();
    }

    /// @notice Attacker cannot transfer ownership
    function test_transferOwnership_onlyOwner() public {
        vm.prank(attacker);
        vm.expectRevert("Not owner");
        token.transferOwnership(attacker);
    }

    /// @notice Owner can transfer ownership successfully
    function test_transferOwnership_succeeds() public {
        address newOwner = makeAddr("newOwner");
        vm.prank(owner);
        token.transferOwnership(newOwner);
        assertEq(token.owner(), newOwner);
    }

    /// @notice tx.origin bypass fails with msg.sender check
    function test_txOriginBypass_fails() public {
        // Simulate owner calling a phishing contract
        // tx.origin = owner, msg.sender = phishingContract
        PhishingContract phisher = new PhishingContract(address(token));
        vm.prank(owner);  // owner initiates the call chain
        vm.expectRevert("Not owner");
        phisher.attack();
    }

    /// @notice Fuzz: no random address can gain ownership
    function testFuzz_onlyOwnerCanTransfer(address caller) public {
        vm.assume(caller != owner);
        vm.prank(caller);
        vm.expectRevert();
        token.transferOwnership(caller);
    }
}

Key Takeaways

Every state-changing function needs an explicit access check. Never rely on "it's obviously not supposed to be called by anyone" — make that assumption code.
Never use tx.origin for authorization. It is always bypassable through a phishing intermediary contract. Use msg.sender.
Proxy contracts must call _disableInitializers() in the implementation constructor. If the implementation's initialize() is callable, an attacker can take control of the implementation and potentially brick all proxies pointing to it.
Apply nonReentrant and access modifiers to all related functions. Partial protection often creates worse attack surfaces than no protection because it gives false confidence.
Admin keys should be multisigs with timelocks. A single compromised EOA should not be able to drain a protocol. Ronin Bridge's $625M loss was one private key breach.
Audit initializer functions first. They are the most commonly forgotten access control surface in upgradeable contract codebases.

📚

Reference

SWC-106: Unprotected SELFDESTRUCT

→

🛡️

Documentation

OpenZeppelin Access Control Docs

→

📰

Post-Mortem

Parity Wallet Hack — OZ Blog

→

📰

Post-Mortem

Ronin Bridge $625M Hack Report

→

Role Hierarchy and Admin Key Management

Even correctly implemented access control fails when admin private keys are compromised. The Ronin Bridge attack ($625M) exploited social engineering to compromise validator keys — the code was correct but the key management was not. Access control is only as strong as the key management behind it.

Multi-Signature and Timelock Defense Pattern

// ✅ Best practice: Critical operations require multisig + timelock
contract GovernedProtocol is AccessControl {
    bytes32 public constant ADMIN_ROLE = keccak256("ADMIN_ROLE");
    bytes32 public constant OPERATOR_ROLE = keccak256("OPERATOR_ROLE");
    bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

    TimelockController public immutable timelock;

    constructor(address multisig, address timelockAddress) {
        timelock = TimelockController(payable(timelockAddress));

        // ✅ Admin role is held by Timelock (which is controlled by multisig)
        // To change admin: multisig proposes → 48h delay → execution
        // No single EOA can make instant admin changes
        _grantRole(ADMIN_ROLE, timelockAddress);
        _grantRole(DEFAULT_ADMIN_ROLE, timelockAddress);

        // ✅ Operator can pause/unpause but cannot change critical parameters
        _grantRole(OPERATOR_ROLE, multisig);
        _grantRole(PAUSER_ROLE, multisig);

        // ✅ Revoke deployer's admin — deployment address has no ongoing power
        _revokeRole(DEFAULT_ADMIN_ROLE, msg.sender);
    }

    /// Pausable without timelock — emergency response must be fast
    function pause() external onlyRole(PAUSER_ROLE) { _pause(); }

    /// Parameter changes require timelock — governance decisions need delay
    function setFee(uint256 newFee) external onlyRole(ADMIN_ROLE) {
        require(newFee <= 1000, "Fee too high"); // Sanity check
        fee = newFee;
    }
}

Access Control Audit Checklist

Check	Question	Red Flag
No modifiers	Do all state-changing functions have access control?	Public function with no modifier on admin action
tx.origin usage	Is tx.origin used for authorization?	require(tx.origin == owner) anywhere
Deployer key	Does the deployer retain admin after deployment?	Constructor grants DEFAULT_ADMIN_ROLE to msg.sender permanently
Upgrade authorization	Who can upgrade the proxy?	_authorizeUpgrade with empty body or missing
Initialization	Can initialize() be called after deployment?	Missing initializer modifier on upgradeable contract
Key management	Is the admin a multisig with time delay?	Admin is a single EOA with no timelock
Role separation	Are emergency and governance roles separate?	Same key controls pause and parameter changes
Renouncement risk	Can owner accidentally renounce, locking funds?	renounceOwnership available with no safeguard

Key Takeaways

Every state-changing function must have explicit access control — the absence of a modifier is a vulnerability, not an oversight.
Never use tx.origin for authorization — it is vulnerable to phishing via malicious intermediary contracts. Use msg.sender.
Upgradeable contracts must protect the initialize() function with OpenZeppelin's initializer modifier — the Parity hack froze $150M by calling an unprotected initWallet.
Admin keys should be multisigs with hardware wallets; critical operations should go through a Timelock so users can exit before changes take effect.
Role separation matters — emergency pause should be fast (no timelock), but parameter changes should have a delay for user protection.
After deployment, the deployer EOA should retain no special privileges — all admin power should flow through governance or a multisig.

📘

Documentation

OpenZeppelin Access Control Guide

→

📘

Documentation

OpenZeppelin Defender — Timelock Controller

→

← Reentrancy Attacks Integer Arithmetic Bugs →