Missing Existence Check in Delegatecall Leads to Misleading Execution

Summary

The Proxy2Step contract does not check the existence of the implementation contract before making a delegatecall. This absence of verification could lead to the EVM returning true even when no code is present at the implementation address, potentially causing misleading transaction results and silent failures.

Vulnerability Detail

In a transparent proxy setup like Proxy2Step, the fallback function delegates execution to the implementation contract specified by the implementation storage slot. However, this function does not perform any check to confirm that the address stored in implementation points to an actual contract. According to the EVM design, delegatecall will return true if it is made to a non-existent address, which can lead to the false assumption that a call was successful when it wasn't, thus not triggering any revert. This vulnerability can lead to operational failures, as operations assumed to be successful could effectively have failed without executing any code.

Impact

This could lead to silent failures where calls are assumed to be successful but have no effect, potentially leading to financial losses or functional breakdowns in the DeFi protocol, especially in cases where critical logic depends on the results of delegated calls.

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/proxies/Proxy2Step.sol#L15

Tool used

Manual Review

Recommendation

To address this issue, it's recommended to add a check for the existence of the contract at the implementation address before performing the delegatecall. This can be done using the extcodesize opcode to ensure the target address contains code.

fallback() external virtual payable {
+   address _impl = implementation;
    assembly {
+       if iszero(extcodesize(_impl)) { revert(0, 0) }

        calldatacopy(0, 0, calldatasize())
+       let result := delegatecall(gas(), _impl, 0, calldatasize(), 0, 0)
-           let result := delegatecall(gas(), sload(implementation.slot), 0, calldatasize(), 0, 0)
        returndatacopy(0, 0, returndatasize())
        switch result
        case 0 { revert(0, returndatasize()) }
        default { return(0, returndatasize()) }
    }
}

sherlock-audit / 2024-05-sophon-judging

dimi6oni - Missing Existence Check in Delegatecall Leads to Misleading Execution #203