Broken access control

Vulnerable code

address public owner;

function setOwner(address newOwner) external {
    owner = newOwner;              // anyone can call this
}

function emergencyWithdraw() public {
    payable(msg.sender).transfer(address(this).balance);
}

Why it breaks

Neither function has a modifier. Any address can call setOwner to take ownership, or skip that entirely and call emergencyWithdraw to empty the contract. This looks obvious in isolation, but in a 2,000-line codebase with forty functions it is genuinely easy to miss one, particularly during a refactor where a modifier gets dropped from a function signature.

Watch the attack run

The same bug as a stepped sequence. Play it, or walk through with the arrow keys.

The fix

address public owner;

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

function setOwner(address newOwner) external onlyOwner {
    require(newOwner != address(0), "zero address");
    owner = newOwner;
}

function emergencyWithdraw() external onlyOwner {
    payable(owner).transfer(address(this).balance);
}

The invariant

The fix above is what an audit gives you. This is what monitoring gives you: the thing that should always be true once the contract is live, stated precisely enough to check every block.

invariant  owner(t) == owner(t-1)
  unless tx in expected_governance_set

watch  OwnershipTransferred, RoleGranted, RoleRevoked

How to test for it

A finding you cannot reproduce is an opinion. Write the test before you write the fix, watch it fail, then make it pass.

function testOnlyOwnerCanSetOwner() public {
    vm.prank(attacker);
    vm.expectRevert("not owner");
    target.setOwner(attacker);
}

// Write one of these for EVERY privileged function.
// Missing tests are how missing modifiers survive to mainnet.

Where it has caused real losses

Access control failures account for a large share of total value lost across Web3. The Poly Network incident in 2021, roughly $611M, came down to a cross-chain contract accepting a crafted message that let the attacker change the keeper: an authorisation check that did not do what its authors believed it did.

What we check during review

Two patterns worth flagging in review. First, ownership transfer without a two-step handshake: a single-transaction transferOwnership to a mistyped address bricks the contract permanently. Use a propose/accept pair. Second, initialiser functions on upgradeable contracts that can be called by anyone if left unprotected, see our page on uninitialised proxies.