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

6 stars 4 forks source link

QA/Low report #229

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

QA/Low report

Low/Info issue submitted by TheSeraphs

Summary: Missing 0 value check for immutable variables

Contract: OptimismPortal2.sol

Vulnerability Detail

Both of these variables are critical for messages sent from L2 to L1 via the OptimismPortal2 contract.

The message can be executed PROOF_MATURITY_DELAY_SECONDS after the withdrawal has been proven and DISPUTE_GAME_FINALITY_DELAY_SECONDS after its corresponding proposal (located within a FaultDisputeGame) was finalized.

Impact

Considering the significance of protocol integrity, despite the low probability of risk, it is essential to adhere to the protocol's precautionary principles, adding an essential layer of safety. To this end, it is imporant to implement checks for two immutable variables to guarantee that delays are appropriately enforced within the protocol. Specifically, these checks should verify that PROOF_MATURITY_DELAY_SECONDS is applied to the finalization of withdrawals, and DISPUTE_GAME_FINALITY_DELAY_SECONDS is utilized for the resolution and finalization of proposals. By implementing a check that a 0 value is not attributed to either of the varaibles upon deployment of the contract

Code snippet

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

Tool used

Manual Review

Recommendation:

constructor(
uint256 _proofMaturityDelaySeconds,
uint256 _disputeGameFinalityDelaySeconds,
GameType _initialRespectedGameType
) {
+       if (_proofMaturityDelaySeconds == 0 || _disputeGameFinalityDelaySeconds == 0) revert InvalidDelay();
PROOF_MATURITY_DELAY_SECONDS = _proofMaturityDelaySeconds;
DISPUTE_GAME_FINALITY_DELAY_SECONDS = _disputeGameFinalityDelaySeconds;
respectedGameType = _initialRespectedGameType;