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

6 stars 4 forks source link

bareli - wrong implement of "findLatestGames" function. #181

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

bareli

medium

wrong implement of "findLatestGames" function.

Summary

"_n" can be less than start ,there is no check on that ,we will receive less than _n games.There is also no check on whether _disputeGameList array has "_n" games or not.

Vulnerability Detail

function findLatestGames( GameType _gameType, uint256 _start, uint256 n ) external view returns (GameSearchResult[] memory games) { // If the _start index is greater than or equal to the game array length or _n == 0, return an empty array. if (_start >= _disputeGameList.length || n == 0) return games;

    // Allocate enough memory for the full array, but start the array's length at `0`. We may not use all of the
    // memory allocated, but we don't know ahead of time the final size of the array.
    assembly {
        games_ := mload(0x40)
        mstore(0x40, add(games_, add(0x20, shl(0x05, _n))))
    }

    // Perform a reverse linear search for the `_n` most recent games of type `_gameType`.

@> for (uint256 i = _start; i >= 0 && i <= _start;) { GameId id = _disputeGameList[i]; (GameType gameType, Timestamp timestamp, IDisputeGame proxy) = id.unpack();

        if (gameType.raw() == _gameType.raw()) {
            // Increase the size of the `games_` array by 1.
            // SAFETY: We can safely lazily allocate memory here because we pre-allocated enough memory for the max
            //         possible size of the array.
            assembly {
                mstore(games_, add(mload(games_), 0x01))
            }

            bytes memory extraData = proxy.extraData();
            Claim rootClaim = proxy.rootClaim();
            games_[games_.length - 1] = GameSearchResult({
                index: i,
                metadata: id,
                timestamp: timestamp,
                rootClaim: rootClaim,
                extraData: extraData
            });
            if (games_.length >= _n) break;
        }

        unchecked {
            i--;
        }
    }
}

Impact

we will get less than _n latest game. If _disputeGameList array is empty and start is too high then we are wasting a loss of Gas.

Code Snippet

https://github.com/sherlock-audit/2024-02-optimism-2024/blob/main/optimism/packages/contracts-bedrock/src/dispute/DisputeGameFactory.sol#L138

Tool used

Manual Review

Recommendation

function findLatestGames( GameType _gameType, uint256 _start, uint256 n ) external view returns (GameSearchResult[] memory games) { // If the _start index is greater than or equal to the game array length or _n == 0, return an empty array. if (_start >= _disputeGameList.length || n == 0) return games; ++ if(disputeGameList.length==0||disputeGameList.length<n) ++ return games; ++ (start<n) ++ return games;

    // Allocate enough memory for the full array, but start the array's length at `0`. We may not use all of the
    // memory allocated, but we don't know ahead of time the final size of the array.
nevillehuang commented 7 months ago

Invalid, view function not used anywhere else in protocol.