nirohgo - The FGD l2BlockNumber (passed in extraData) can be any number, enabling a DOS on fund widthdrawals

[sherlock-admin4]

nirohgo

high

The FGD l2BlockNumber (passed in extraData) can be any number, enabling a DOS on fund widthdrawals

Summary

A game's l2BlockNumber value is not checked to match the actual L2 block number being Claimed, which enables a DOS by creating and succsesfully resolving a game based on a valid rootClaim but with a uint256.max block number, preventing further games from running.

Vulnerability Detail

In the new Fault Proof system, games can only be created if the L2 block number they attempt to prove is larger than the last Anchor block of the same GameType. At the same time, when a game is created, the L2 block number it tries to prove is speficied in two places: 1. the latestBlockhash field of OutputRootProof (hashed into the the rootClaim Claim) 2. the extraData parameter. the first is used as part of the game execution process, while the second is used to identify the game's rootClaim block when checking that it is not earlier than the current anchor, and when setting the game as the new anchor (if it is resolved).

The root cause of this issue is that there is no validity check on the extraData block number and technically any block number can be used. This enables the following attack:

1. Starting state: The current anchor block number for respected-game-type is X, and the L2 current block is X+5.
2. Attacker starts a dispute game for block X+5 with a valid rootClaim, but with type(uint256).max block number in extraData.
3. The game can not be succesfully challanged (because its rootClaim is valid) and is resolved by GAME_DURATION at most.
4. Since the game was created immediately after L2 block X+5 is finalized, it is the first game proving X+5 to resolve, and when resolved it is set at the new Anchor in AnchorStateRegistry, as block number type(uint256).max.
5. From this point on there is no way to create any new games, since they will all fail the check in FaultDisputeGame.sol line 539:

(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());

As a result, no withdrawal transaction that enters the L2 chain at a block larger then X+5 can be proven nor finalized (since no game can exist that proves its state)

6. Note that blacklisting the game would not have any effect in this case because it only affects the ability to prove withdrawals based on the blacklisted game, but it will not prevent the game from resolving nor will it unset it from the AnchorStateRegistry when it resolves.
7. The only way to resolve this issue would be to redeploy the system with a fix, which given the complexity of the system and time of root cause discovery+analysis+writing a fix+testing+auditing, would take well over a week.

POC

1. In test file test/dispute/FaultDisputeGame.t.sol change line 95 from

super.init({ rootClaim: ROOT_CLAIM, absolutePrestate: absolutePrestate, l2BlockNumber: 0x10 });

to

super.init({ rootClaim: ROOT_CLAIM, absolutePrestate: absolutePrestate, l2BlockNumber: type(uint256).max });

(which sets the extraData parameter to max uint256 without affecting the Claim)
2. run pnpm build:go-ffi && forge test --match-path '*/FaultDisputeGame.t.sol' -vv to see that all Fault Dispute Game tests run succesfully inspite of the change.
3. add the following lines at the end of the test_resolve_validNewerStateUpdatesAnchor_succeeds funcion in test/dispute/FaultDisputeGame.t.sol:

//Try to create a new game at any block number.
//See that is fails with UnexpectedRootClaim 
//(emitted during FaultDisputeGate::initialize when blockNumber is smaller than Anchor)
extraData = abi.encode(type(uint256).max / 2);
vm.expectRevert(abi.encodeWithSelector(UnexpectedRootClaim.selector, ROOT_CLAIM));
gameProxy = FaultDisputeGame(payable(address(disputeGameFactory.create(GAME_TYPE, ROOT_CLAIM, extraData))));

4. run pnpm build:go-ffi && forge test --match-test 'test_resolve_validNewerStateUpdatesAnchor_succeeds' -vv
   See that after the attack, creating a new game fails with any block number.

Impact

A de-facto DOS preventing game creation/withdrawal prove/withdrawal finalization for any blocks and withdrawals following the previous anchor block (block X) that will last well over a week (on top of expected game duration, safety delays etc.). The combined effect is locked funds for over a week and denial of availability of time sensitive functions (game creation, game execution, widthdrawal proof, withdrawal finalization) making this high sevirity based on the Sherlock rule on DOS.

Code Snippet

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

Tool used

Manual Review
Foundry

Recommendation

As part of the game resolution process, validate the L2BlockNumber given in extraData and revert if the number is invalid.

Duplicate of #90