Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bin2chen - initialize() DOS attack by very big l2BlockNumber #44

Closed
sherlock-admin2 opened this issue Apr 4, 2024 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Apr 4, 2024

bin2chen

medium

initialize() DOS attack by very big l2BlockNumber

Summary

Maliciously specifying l2BlockNumber == type (uint256).max may block subsequent Game initialization

Vulnerability Detail

Currently FaultDisputeGame.initialize () will limit not less than or equal to the previous l2BlockNumber

    function initialize() public payable virtual {
...
@>      (Hash root, uint256 rootBlockNumber) = ANCHOR_STATE_REGISTRY.anchors(GAME_TYPE);

        // Should only happen if this is a new game type that hasn't been set up yet.
        if (root.raw() == bytes32(0)) revert AnchorRootNotFound();

        // Set the starting output root.
        startingOutputRoot = OutputRoot({ l2BlockNumber: rootBlockNumber, root: root });

        // Do not allow the game to be initialized if the root claim corresponds to a block at or before the
        // configured starting block number.
@>      if (l2BlockNumber() <= rootBlockNumber) revert UnexpectedRootClaim(rootClaim());

And when DEFENDER_WINS will update root = l2BlockNumber

    function resolve() external returns (GameStatus status_) {
..

        // Update the status and emit the resolved event, note that we're performing an assignment here.
        emit Resolved(status = status_);

        // Try to update the anchor state, this should not revert.
@>      ANCHOR_STATE_REGISTRY.tryUpdateAnchorState();
    }
contract AnchorStateRegistry is Initializable, IAnchorStateRegistry, ISemver {
...
    function tryUpdateAnchorState() external {

    function tryUpdateAnchorState() external {
        // Grab the game and game data.
        IFaultDisputeGame game = IFaultDisputeGame(msg.sender);
        (GameType gameType, Claim rootClaim, bytes memory extraData) = game.gameData();

        // Grab the verified address of the game based on the game data.
        // slither-disable-next-line unused-return
        (IDisputeGame factoryRegisteredGame,) =
            DISPUTE_GAME_FACTORY.games({ _gameType: gameType, _rootClaim: rootClaim, _extraData: extraData });

        // Must be a valid game.
        require(
            address(factoryRegisteredGame) == address(game),
            "AnchorStateRegistry: fault dispute game not registered with factory"
        );

        // No need to update anything if the anchor state is already newer.
        if (game.l2BlockNumber() <= anchors[gameType].l2BlockNumber) {
            return;
        }

        // Must be a game that resolved in favor of the state.
        if (game.status() != GameStatus.DEFENDER_WINS) {
            return;
        }

        // Actually update the anchor state.
@>      anchors[gameType] = OutputRoot({ l2BlockNumber: game.l2BlockNumber(), root: Hash.wrap(game.rootClaim().raw()) });
    }

In this way, the user can submit a l2BlockNumber == type (uint256).max, and it is a correct rootClaim
When DEFENDER_WINS, OutputRoot.root is changed to type (uint256).max
In this way, subsequent Game will revert to UnexpectedRootClaim initialization failure.

Impact

Block subsequent Game initialization

Code Snippet

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

Tool used

Manual Review

Recommendation

cancel <= rootBlockNumber limit

    function initialize() public payable virtual {
...

-       if (l2BlockNumber() <= rootBlockNumber) revert UnexpectedRootClaim(rootClaim());

        // Revert if the calldata size is too large, which signals that the `extraData` contains more than expected.
        // This is to prevent adding extra bytes to the `extraData` that result in a different game UUID in the factory,
        // but are not used by the game, which would allow for multiple dispute games for the same output proposal to
        // be created.
        // Expected length: 0x66 (0x04 selector + 0x20 root claim + 0x20 l1 head + 0x20 extraData + 0x02 CWIA bytes)
        assembly {
            if gt(calldatasize(), 0x66) {
                // Store the selector for `ExtraDataTooLong()` & revert
                mstore(0x00, 0xc407e025)
                revert(0x1C, 0x04)
            }
        }

Duplicate of #90

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 11, 2024
@sherlock-admin2 sherlock-admin2 added Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed labels Apr 11, 2024
@sherlock-admin3 sherlock-admin3 changed the title Vast Currant Parrot - initialize() DOS attack by very big l2BlockNumber bin2chen - initialize() DOS attack by very big l2BlockNumber Apr 23, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Apr 23, 2024
@Evert0x Evert0x added Medium A valid Medium severity issue and removed High A valid High severity issue labels May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

3 participants