sherlock-audit / 2024-02-optimism-2024-judging

6 stars 4 forks source link

QA/Low report #228

Closed sherlock-admin3 closed 7 months ago

sherlock-admin3 commented 7 months ago

QA/Low report

Low/Info issue submitted by TheSeraphs

Summary: Missing 0 address check

Contract: OptimismPortal2.sol

Vulnerability Detail

The initialize function is missing a address(0) check for all 3 main input contracts addresses.

    function initialize(
        DisputeGameFactory _disputeGameFactory,
        SystemConfig _systemConfig,
        SuperchainConfig _superchainConfig
    )
        public
        initializer
    {
        disputeGameFactory = _disputeGameFactory; 
        systemConfig = _systemConfig; 
        superchainConfig = _superchainConfig; 
        if (l2Sender == address(0)) {
            l2Sender = Constants.DEFAULT_L2_SENDER;
        }
        __ResourceMetering_init();
    }

Impact

Even though the likelihood is low, it's good practice to include checks for inputs that could cause unexpected behaviour within protocols; be it in constructors or initialization functions; especially those that could impact the protocol functionality upon deployment.

Code snippet

https://github.com/sherlock-audit/2024-02-optimism-2024/blob/main/optimism/packages/contracts-bedrock/src/L1/OptimismPortal2.sol#L147C1-L162C6

Tool used

Manual Review

Recommendation:


    function initialize(
        DisputeGameFactory _disputeGameFactory,
        SystemConfig _systemConfig,
        SuperchainConfig _superchainConfig
    )
        public
        initializer
    {
+       if (_disputeGameFactory == address(0) || _systemConfig == address(0) || _superchainConfig == address(0)) revert InvalidContractAddress(); 
        disputeGameFactory = _disputeGameFactory; 
        systemConfig = _systemConfig; 
        superchainConfig = _superchainConfig; 
        if (l2Sender == address(0)) {
            l2Sender = Constants.DEFAULT_L2_SENDER;
        }
        __ResourceMetering_init();
    }
``