Upgradeable contracts: storage collisions, uninitialised proxies and admin risk

Upgradeability is attack surface, not a safety net

Teams add upgradeability to be able to fix bugs. The trade is rarely stated plainly: you gain the ability to fix problems, and you take on a delegatecall relationship, a storage-layout contract that the compiler will not check for you, and an admin key that can replace your entire codebase.

That trade is often correct. But it means the upgrade mechanism is itself part of what needs reviewing — and in our experience, the proxy is the least-tested part of most upgradeable systems, because it is boilerplate everyone assumes is safe.

The three patterns in common use fail differently, so it is worth being precise about which one you are reviewing.

Transparent, UUPS and Diamond, by failure mode

Transparent proxy. The proxy itself decides: calls from the admin go to admin functions, everything else is delegated. Upgrade logic lives in the proxy, which makes the proxy larger and every user call slightly more expensive. Its characteristic failure is function-selector shadowing — if the implementation has a function whose selector collides with an admin function, the admin can never call it through the proxy.

UUPS. The upgrade function lives in the implementation, which makes the proxy minimal and cheap. Its characteristic failure is catastrophic: if a new implementation omits _authorizeUpgrade, or omits the upgrade mechanism entirely, the proxy becomes permanently unupgradeable. And if the implementation contract is left initialisable, an attacker can initialise it directly, become its owner, and call upgradeToAndCall to selfdestruct it — bricking every proxy that points at it.

Diamond (EIP-2535). Many implementation contracts ("facets") behind one proxy, with a selector-to-facet routing table. Its characteristic failures are selector collisions between facets and a storage model that must be managed by hand across every facet. Powerful, and the hardest of the three to review — the effective attack surface is the union of every facet plus the routing table itself.

Storage layout: the contract the compiler will not check

The proxy holds the storage; the implementation defines its layout. Change the layout between versions and the new code reads old data at the wrong slots — silently, with no revert.

// V1
contract VaultV1 {
    address public owner;      // slot 0
    uint256 public totalShares; // slot 1
    mapping(address => uint256) public balanceOf;  // slot 2
}

// V2 — BROKEN: inserting a variable shifts everything after it
contract VaultV2 {
    address public owner;       // slot 0
    address public treasury;    // slot 1  <-- now reads old totalShares
    uint256 public totalShares; // slot 2  <-- now reads the mapping's base slot
    mapping(address => uint256) public balanceOf;  // slot 3
}

// V2 — CORRECT: append only
contract VaultV2 {
    address public owner;
    uint256 public totalShares;
    mapping(address => uint256) public balanceOf;
    address public treasury;    // new variables go at the end
}

The rules, in full:

• Append only. Never insert, reorder or remove a state variable. To retire one, leave it in place and rename it __deprecated_x.
• Never change a type's size or packing. Changing uint128 to uint256 unpacks a slot and shifts everything after it.
• Inheritance order is layout order. Adding or reordering base contracts changes slots even if your own declarations are untouched.
• Use storage gaps in upgradeable base contracts: uint256[50] private __gap; so children can grow without collision.
• Prefer namespaced storage (ERC-7201) for new work — each module gets a pseudorandom base slot and layout collisions become structurally impossible.
• Verify mechanically. forge inspect Contract storageLayout before and after, diffed in CI. OpenZeppelin's upgrades plugin does this automatically; do not rely on a human reading two files side by side.

Initializers, implementations and the front-running window

Constructors do not run in the proxy's context, so initialisation is a regular function. Three things must be true, and audits find at least one missing more often than not:

contract Vault is UUPSUpgradeable, OwnableUpgradeable {

    /// 1. The implementation itself must never be initialisable.
    constructor() {
        _disableInitializers();
    }

    /// 2. The initializer must be guarded and callable exactly once.
    function initialize(address owner_) external initializer {
        require(owner_ != address(0), "zero owner");
        __Ownable_init(owner_);
        __UUPSUpgradeable_init();
    }

    /// 3. Upgrade authority must be explicit, and must exist in every
    ///    future implementation — omit it and the proxy is frozen forever.
    function _authorizeUpgrade(address) internal override onlyOwner {}

    /// 4. New state introduced in V2 needs a reinitializer, not a re-run.
    function initializeV2(address treasury_) external reinitializer(2) {
        treasury = treasury_;
    }
}

The fourth item on the deployment side: deploy and initialise atomically. A proxy deployed in one transaction and initialised in the next has a window in which anyone can call initialize and take ownership. Use a factory, or pass the initialisation calldata to the proxy constructor.

delegatecall, and what it lets someone else do to you

delegatecall runs foreign code against your storage, your balance and your address. In a proxy that is the point. Everywhere else it is a question worth asking twice.

// CRITICAL — a user-supplied target can rewrite any storage slot,
// including the implementation pointer and the owner.
function execute(address target, bytes calldata data) external {
    (bool ok, ) = target.delegatecall(data);
    require(ok);
}

Multicall implementations that delegatecall into themselves deserve a second look too: combined with a payable function that reads msg.value, the same value can be counted once per sub-call in a single transaction — a bug that has shipped in production more than once.

For any delegatecall in a codebase, we require: an immutable or allowlisted target, a documented and verified storage-layout compatibility with the caller, and a written justification for why a regular call is insufficient.

The upgrade key is the protocol

An upgradeable protocol's security ceiling is the security of whoever can upgrade it. Everything below is a review requirement, not a nice-to-have:

• Timelock every upgrade. Long enough for users to exit — 48 hours is a common floor. An instant upgrade path means user funds are custodial in practice, whatever the marketing says.
• Multisig with a real threshold and independent signers. A 2-of-3 where all three keys are on one person's laptop is a 1-of-1 wearing a disguise.
• Separate upgrade authority from operational keys. The keeper key that signs twenty transactions a day should not be able to replace the implementation.
• Emergency pause must only make things safer. A pause that blocks withdrawals but permits deposits is a hostage function.
• Review every upgrade transaction before it is signed — the new implementation's storage layout, the initialiser, and the calldata itself. Most upgrade incidents are operational, not cryptographic.
• Publish the timelock queue. Users cannot exercise an exit window they cannot see.

Proxy review checklist

• Identify the pattern precisely (Transparent / UUPS / Beacon / Diamond) and confirm the implementation matches the pattern's requirements.
• _disableInitializers() in the implementation constructor — verify on-chain, not just in the source.
• Initializer guarded with initializer; every subsequent version uses reinitializer(n).
• Deployment and initialisation are atomic. Read the deployment script.
• Storage layout diffed mechanically between versions; gaps present in upgradeable bases.
• _authorizeUpgrade present and correctly guarded in every implementation, including future ones.
• Every delegatecall target is immutable or allowlisted.
• Upgrade authority is timelocked, multisig-held, and separate from operational keys.
• Selector collisions checked (Transparent: admin functions; Diamond: across all facets).
• The full admin capability list is written down and compared against what users have been told.

Upgradeability is a reasonable engineering decision and a serious security commitment. If you take it, review the mechanism with the same rigour as the logic it is meant to protect — because an attacker who owns your upgrade path does not need to find any other bug.

All code in this article is illustrative — written to demonstrate a pattern, not taken from any client engagement. If you want this class of issue hunted in your own codebase, that is what a Smart Contract Security Audit is for.