Unsafe delegatecall

Vulnerable code

address public owner;
address public library_;

function execute(bytes calldata data) external {
    (bool ok, ) = library_.delegatecall(data);
    require(ok);
}

function setLibrary(address l) external {
    library_ = l;             // unprotected — attacker points it anywhere
}

Why it breaks

An attacker calls setLibrary with the address of a contract they wrote, then calls execute with calldata for a function that sets slot 0. Slot 0 is owner. They now own the contract. Even with setLibrary protected, a delegatecall to a library whose storage layout differs from yours will corrupt state: the library's first variable writes over your owner.

Watch the attack run

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

The fix

address public immutable library_;   // fixed at deploy

constructor(address l) { library_ = l; }

function execute(bytes calldata data) external onlyOwner {
    require(_isAllowedSelector(bytes4(data)), "selector not allowed");
    (bool ok, ) = library_.delegatecall(data);
    require(ok);
}

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  sload(EIP1967_IMPL_SLOT)(t)
           == sload(EIP1967_IMPL_SLOT)(t-1)
  unless tx in expected_upgrade_set

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 testCannotHijackViaDelegatecall() public {
    Evil evil = new Evil();
    vm.prank(attacker);
    vm.expectRevert();
    proxy.setLibrary(address(evil));

    assertEq(proxy.owner(), owner);   // ownership intact
}

Where it has caused real losses

The second Parity multisig incident in 2017 froze roughly 513,000 ETH permanently. A shared library contract was left uninitialised; a user claimed ownership of it and then called selfdestruct. Every wallet that delegatecalled into that library became unusable, and the funds inside are still inaccessible.

What we check during review

Rules we apply in review: the delegatecall target should be immutable or governance-controlled with a timelock, never settable by an arbitrary caller. Storage layouts between proxy and implementation must match exactly: use unstructured storage slots (EIP-1967) rather than hoping declaration order stays aligned across upgrades. And any library that can be called directly must have its own initialiser locked in the constructor.