Access control gone wrong: who can call this function?

A door with no lock

On 19 July 2017, an attacker emptied three Parity multisig wallets of about 153,000 ETH — roughly $30 million at the time. Four months later, in November, a curious user poking at the replacement code accidentally froze more than 513,000 ETH — over $150 million — beyond anyone's reach, forever. Neither attack broke any cryptography. No private key was stolen, no hash was reversed. Both were the same mundane failure: a function that should have asked "are you allowed to do this?" simply... didn't.

That question — *who is allowed to call this function?* — is access control, and getting it wrong is one of the most common and most expensive bug classes in smart contracts. You have already met reentrancy and arithmetic bugs in this rung; those are about how code runs. Access control is about who is permitted to run it. In a system where code is public and anyone on Earth can call any function, every privileged action needs a lock, and every lock needs to be on the right door.

The missing modifier

The simplest access-control bug is the one that hides in plain sight: a function that changes who owns the contract, mints tokens, or moves money, with no check at all on the caller. Here is a vault that anyone can take over.

// VULNERABLE — anyone can become owner, then drain the vault
contract Vault {
    address public owner;
    constructor() { owner = msg.sender; }

    function setOwner(address newOwner) public {   // <-- no access check!
        owner = newOwner;
    }

    function withdraw() public {
        require(msg.sender == owner, "not owner");
        payable(owner).transfer(address(this).balance);
    }
}

The `withdraw` function looks safe — it checks `msg.sender == owner`. But that guard is worthless, because `setOwner` lets anyone rewrite `owner` first. The fix is to gate every state-changing privileged function behind an explicit check. Rather than hand-roll it, inherit a battle-tested base like OpenZeppelin's `Ownable`, which gives you an `onlyOwner` modifier and a safe `transferOwnership`.

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

// FIXED — ownership can only move via Ownable's onlyOwner-guarded path
contract Vault is Ownable {
    constructor() Ownable(msg.sender) {}

    function withdraw() external onlyOwner {
        payable(owner()).transfer(address(this).balance);
    }
    // transferOwnership(newOwner) comes from Ownable and is itself onlyOwner
}

tx.origin: the deputy you can trick

Solidity gives you two ways to ask "who called me," and confusing them is a classic trap. `msg.sender` is the immediate caller — the contract or account one hop away. `tx.origin` is the original externally-owned account that started the whole transaction, no matter how many contracts it passed through. Authorizing with `tx.origin` looks equivalent, but it lets an attacker borrow the victim's identity.

Imagine a wallet that checks `tx.origin == owner`. An attacker deploys a contract that looks like a free airdrop and persuades the owner to call it. When the owner does, the attacker's contract calls the victim wallet — and because the owner started the transaction, `tx.origin` is still the owner. The check passes; the attacker drains the wallet. This is tx.origin phishing.

// VULNERABLE wallet — authorizes by tx.origin
contract Wallet {
    address public owner = msg.sender;
    function transferTo(address payable to, uint256 amount) public {
        require(tx.origin == owner, "not owner");   // <-- BUG
        to.transfer(amount);
    }
}

// Attacker's bait: the owner is tricked into calling attack()
contract Phish {
    Wallet immutable victim;
    address payable immutable attacker;
    constructor(Wallet _victim) {
        victim = _victim;
        attacker = payable(msg.sender);
    }
    function attack() external {                     // looks like "claim reward"
        victim.transferTo(attacker, address(victim).balance);
        // msg.sender here is Phish, but tx.origin is still the owner -> passes
    }
}

The fix is one word: authorize with `msg.sender`, never `tx.origin`. `require(msg.sender == owner)` would reject `Phish`, because the immediate caller is the attacker's contract, not the owner. `tx.origin` has a few legitimate niches, but for authorization it is simply wrong.

The Parity disaster: initialize once, or lose everything

Now we can dissect the Parity hacks properly, because they combine missing access control with the proxy pattern you met at the end of the Solidity rung. To save gas, each Parity multisig wallet was a thin proxy that held the funds but delegatecalled all its logic into one shared library contract. Crucially, `delegatecall` runs the library's code in the wallet's own storage — so the library's `initWallet` function set the wallet's owners.

// Simplified Parity WalletLibrary — delegatecalled by every wallet
contract WalletLibrary {
    address public owner;

    // meant to run ONCE at wallet creation — but nothing enforces that
    function initWallet(address _owner) public {
        owner = _owner;               // <-- re-ownable by anyone, anytime
    }

    function kill(address payable to) public {
        require(msg.sender == owner, "not owner");
        selfdestruct(to);             // removes this contract's code
    }
}

First hack (July 2017). An attacker called `initWallet` on existing, funded wallets through the proxy. Because nothing stopped `initWallet` from running a second time, the attacker reset themselves as the wallet's sole owner and walked away with about 153,000 ETH (~$30M). The White Hat Group then used the very same trick to rescue the remaining at-risk funds before others could.

Second incident (November 2017). The patched library still had one fatal gap: the library contract itself was never initialized. A user known as *devops199* called `initWallet` directly on the library, making themselves its owner, then called `kill`. Walk the consequence:

1. initWallet(devops199) on the library contract — it had no owner yet, so this succeeds and devops199 becomes the library's owner.
2. kill(devops199) passes the msg.sender == owner check and runs selfdestruct, erasing the library's bytecode from the chain.
3. Every Parity multisig delegatecalled into that address for its logic — but the code is now gone, so withdraw, transfer, every function reverts.
4. Result: more than 513,000 ETH — over $150 million at the time, and often cited near $300M at later prices — frozen permanently. Nothing was stolen; it was simply bricked, with no way to upgrade out.

The root cause of both is the same one-line omission: a privileged initializer with no guard, on code that should only ever be initialized once. The fix is an initializer guard — and, on the implementation/library, disabling initialization entirely.

contract WalletLogic {
    address public owner;
    bool private initialized;

    function initialize(address _owner) external {
        require(!initialized, "already initialized");
        initialized = true;           // can never run a second time
        owner = _owner;
    }
}

Uninitialized proxies and the $10M bounty

The Parity freeze was not a one-off; uninitialized implementations remain one of the deadliest mistakes in upgradeable contracts. In the modern UUPS and transparent proxy patterns, a proxy holds the storage and funds while delegatecalling logic into a separate implementation contract. The implementation is only ever supposed to run via the proxy — but it is a deployed contract too, and if its `initialize()` is left callable, an attacker can initialize the implementation, become its owner, and (in UUPS) call its upgrade hook to `selfdestruct` it. Every proxy pointing at it is then bricked, Parity-style.

This is not theoretical. In February 2022, a white-hat reported exactly this in the Wormhole bridge: its core implementation contract was uninitialized, so anyone could initialize and then self-destruct it, freezing the bridge. Wormhole paid a $10 million bug bounty — among the largest ever — for the report, and the bug was fixed before it could be exploited.

OpenZeppelin's upgradeable contracts encode the fix directly. The `initializer` modifier makes `initialize` run at most once on the proxy; and `_disableInitializers()` in the implementation's constructor permanently locks the implementation so it can never be initialized at all.

import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract Vault is Initializable, OwnableUpgradeable, UUPSUpgradeable {
    /// @custom:oz-upgrades-unsafe-allow constructor
    constructor() {
        _disableInitializers();        // implementation can NEVER be initialized
    }

    function initialize(address admin) public initializer {
        __Ownable_init(admin);         // runs once, on the proxy only
        __UUPSUpgradeable_init();
    }

    // who may upgrade the logic — the most privileged action of all
    function _authorizeUpgrade(address) internal override onlyOwner {}
}

Designing access control that holds

Patching individual bugs is reactive; professional contracts design their authority model up front. Move beyond a single `owner` to role-based access control when different actors need different powers — a minter, a pauser, an upgrader — so a compromise of one role does not hand over everything. OpenZeppelin's `AccessControl` gives each role a `bytes32` id and an admin role that can grant and revoke it.

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

contract Token is AccessControl {
    bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE");

    constructor(address admin) {
        _grantRole(DEFAULT_ADMIN_ROLE, admin);   // can manage all roles
    }

    function mint(address to, uint256 amount) external onlyRole(MINTER_ROLE) {
        _mint(to, amount);
    }
}

Two principles tie it together. First, least privilege: every function should demand the narrowest role that can justify calling it, and powerful keys (admin, upgrader) should sit behind a multisig and a timelock, not a single hot wallet. Second, fail closed: the default for any privileged action is deny, and you add explicit grants — never the reverse.

1. List every state-changing function and ask, out loud, "who is allowed to call this, and what stops everyone else?"
2. Give each one an explicit visibility and the narrowest access check it needs — onlyOwner, onlyRole, or a custom guard.
3. Guard every initializer so it runs once, and call _disableInitializers() on upgradeable implementations.
4. Put admin and upgrade powers behind a multisig + timelock, and prefer two-step ownership transfer.
5. Verify it: write tests that assert unauthorized callers revert, and run a audit / static-analysis pass that flags missing modifiers before mainnet.