sherlock-audit / 2023-01-optimism-judging

24 stars 10 forks source link

RaymondFam - `getL2OutputIndexAfter()` returns output regardless of the validity of input `_l2BlockNumber` #219

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

RaymondFam

medium

getL2OutputIndexAfter() returns output regardless of the validity of input _l2BlockNumber

Summary

In L2OutputOracle.sol, getL2OutputIndexAfter() is supposed to return the index of the L2 output that checkpoints a given L2 block number.

Vulnerability Detail

The issue is that the function getL2OutputIndexAfter() could return an erroneous index, even if _l2BlockNumber does not exist in l2Outputs.

Impact

This could lead to confusion and mess up with future implementations dependent on getL2OutputIndexAfter().

Code Snippet

In the code snippet below, arr is typical of a scenario where the elements between 5 and 11 have been deleted via deleteL2Outputs().

If you were to input an already deleted element, say 7, which is non-existent, it will return index 5. Additionally, it will return index 9 for any element that is greater than 15.

contract Storage {

    uint[] arr = [1, 2, 3, 4, 5, 11, 12, 13, 14, 15];

    function bee() public view returns(uint[] memory) {
        return arr;
    }

   function foo(uint blockNumber) public view returns(uint){
        // Find the output via binary search, guaranteed to exist.
        uint256 lo = 0;
        uint256 hi = arr.length;
        while (lo < hi) {
            uint256 mid = (lo + hi) / 2;
            if (arr[mid] < blockNumber) {
                lo = mid + 1;
            } else {
                hi = mid;
            }
        }

        return lo;
   }

}

Tool used

Manual Review and Remix

Recommendation

Consider using mapping to have _l2BlockNumber mapped to Types.OutputProposal.

rcstanciu commented 1 year ago

Comment from Optimism


Description: getL2OutputIndexAfter() returns output regardless of the validity of input _l2BlockNumber

Reason: This report is false (similar to #240). The getL2OutputIndexAfter function reverts if the provided l2 block number is greater than the latest block number within the L2OutputOracle's l2Outputs array. The intended behavior of the function is that, for all inputs <= the latest block number in the l2Outputs array, the nearest index corresponding to a block number after or at the input block number will be returned. This is because the goal of the function is to find the "index of the first checkpoint that commits to the given L2 block number."