FV-SOL-7 Proxy Insecurities

TLDR

Upgradeability is essential for maintaining and improving deployed contracts and fixes over time

Due to their nature, they are often misunderstood or implemented insecurely

Code

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

// Delegate contract contains logic but no storage
contract Delegate {
    uint public storedData;  // This variable will be ignored when using delegatecall

    // Function to be called via delegatecall
    function setValue(uint _value) public {
        storedData = _value;  // This will set the caller's storage, not Delegate's
    }
}

// Caller contract with storage that will be updated
contract Caller {
    uint public storedData;  // The storage slot used in Delegate

    // Function to execute delegatecall to Delegate contract
    function setDelegateValue(address _delegateAddress, uint _value) public {
        // Prepare data for delegatecall (function selector + argument)
        (bool success, ) = _delegateAddress.delegatecall(
            abi.encodeWithSignature("setValue(uint256)", _value)
        );
        require(success, "Delegatecall failed");
    }
}

Classifications

delegatecall Storage Collision (FV-SOL-7-C1)

delegatecall is a function call that allows a contract to run code from another contract while preserving the original caller's context, including storage, msg.sender, and msg.value

Since delegatecall runs in the storage context of the caller, if the contract calls delegatecall on user-supplied input, an attacker can input an address to a malicious contract that can manipulate the storage of the calling contract, potentially overwriting sensitive variables or stealing funds

If the target contract has a different storage layout, it may overwrite or corrupt crucial storage variables in the calling contract

Function Selector Collision (FV-SOL-7-C2)

When two functions in the implementation contract have the same function selector, unintended functions can be called, leading to incorrect behavior or security loopholes

Centralized Update Control (FV-SOL-7-C3)

If the upgrade process is too centralized, it creates a single point of failure, and generaly considered unethical to the users

Uninitialized Proxy (FV-SOL-7-C4)

If the initializer function isn’t called during deployment, it may leave critical variables in an unprotected state

Same goes for accidentally allowing initializer functions to be called again in the proxy pattern which can result in re-initializing the contract, doing so gaining ability to modify data

Mitigation Patterns

Validate Addresses Being Called (FV-SOL-7-M1)

Ensure that the address used with delegatecall is fixed or restricted to trusted sources

Limit State Changes (FV-SOL-7-M2)

Be cautious of contracts that use delegatecall to avoid unintended storage changes

__gap Array (FV-SOL-7-M3)

The __gap variable is a common technique used in Solidity's upgradeable contract design to prevent storage layout issues during contract upgrades. It is essentially a reserved area in the contract's storage layout that provides "padding" for future storage variables

Actual Occurrences

• https://solodit.cyfrin.io/issues/h-03-attacker-can-gain-control-of-counterfactual-wallet-code4rena-biconomy-biconomy-smart-contract-wallet-contest-git

• https://solodit.cyfrin.io/issues/h01-corruptible-storage-upgradeability-pattern-openzeppelin-ribbon-finance-audit-markdown

• https://solodit.cyfrin.io/issues/diamond-proxy-initialize-functions-can-be-called-multiple-times-halborn-polemos-lending-pdf

Content

https://www.halborn.com/blog/post/delegatecall-vulnerabilities-in-solidity