code-423n4 / 2024-05-olas-validation

0 stars 0 forks source link

Lack of Compatibility Checks in Implementation of Upgrades #287

Open c4-bot-5 opened 4 months ago

c4-bot-5 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/registries/contracts/staking/StakingProxy.sol#L23

Vulnerability details

root of the bug :

the bug is from the absence of a compatibility check when setting or upgrading the implementation contract. The StakingProxy uses the delegatecall method to forward function calls to the implementation contract stored at a predefined slot in storage. While delegatecall enables the proxy to execute functions defined in the implementation contract, it operates under the assumption that the state layout and logic of the new implementation are compatible with the existing state of the proxy. here this is defines a unique storage slot for storing the implementation address.

bytes32 public constant SERVICE_STAKING_PROXY = 0x9e5e169c1098011e4e5940a3ec1797686b2a8294a9b77a4c676b121bdc0ebb5e;

and here The constructor initializes the implementation address, ensuring it is not zero. However, it does not verify if the new implementation is compatible with the existing state variables of the proxy.

constructor(address implementation) {
    if (implementation == address(0)) {
        revert ZeroImplementationAddress();
    }
    assembly {
        sstore(SERVICE_STAKING_PROXY, implementation)
    }
}

and here The fallback function delegates all calls to the implementation contract but does not check if the implementation’s state is consistent with the proxy’s state.

fallback() external payable {
    assembly {
        let implementation := sload(SERVICE_STAKING_PROXY)
        calldatacopy(0, 0, calldatasize())
        let success := delegatecall(gas(), implementation, 0, calldatasize(), 0, 0)
        returndatacopy(0, 0, returndatasize())
        if eq(success, 0) {
            revert(0, returndatasize())
        }
        return(0, returndatasize())
    }
}

Impact

the bug in the contract is can lead to Loss of Data Integrity cause If the proxy is upgraded to a new implementation that has different state variables or a different state layout, it can result in data corruption

Tools Used

manual review

Recommended Mitigation Steps

it's need to ensure that every upgrade is involves a compatibility check and a safe state migration process.

Assessed type

Other