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

3 stars 0 forks source link

Losing Defender Can Save Bond By Challenging Themselves #90

Closed howlbot-integration[bot] closed 3 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#L652 https://github.com/code-423n4/2024-07-optimism/blob/70556044e5e080930f686c4e5acde420104bb2c4/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L587

Vulnerability details

Impact

Not the same as the known issue

If a defender is going to lose the game, they can call challengeRootL2Block() on themselves, and they will get priority and take their own top level bond payout, instead of paying them to the real challenger.

Background:

To summarize how bonds are payed in the dispute game, in a sub game, If the parent was not successfully countered, the game pays out the parent's bond to the claimant

And If the parent was successfully countered, pay out the parent's bond to the challenger.

This leads to a system where bonds are moved along a chain of subgames, and only the final subgame bonder receives their bonded stake and the bond of the previous challenger

You can see this system by looking at the lines of code where _distributeBond is called

1)

 // If the parent was not successfully countered, pay out the parent's bond to the claimant.
                // If the parent was successfully countered, pay out the parent's bond to the challenger.
                _distributeBond(countered == address(0) ? subgameRootClaim.claimant : countered, subgameRootClaim);

2)

   if (challengeIndicesLen == 0 && _claimIndex != 0) {
            // In the event that the parent claim is at the max depth, there will always be 0 subgames. If the
            // `counteredBy` field is set and there are no subgames, this implies that the parent claim was successfully
            // stepped against. In this case, we pay out the bond to the party that stepped against the parent claim.
            // Otherwise, the parent claim is uncontested, and the bond is returned to the claimant.
            address counteredBy = subgameRootClaim.counteredBy;
            address recipient = counteredBy == address(0) ? subgameRootClaim.claimant : counteredBy;
            _distributeBond(recipient, subgameRootClaim);
            resolvedSubgames[_claimIndex] = true;
            return;
        }

In this method of attack, if a defender knows they are going to lose, they can simply call attack on their false claim themselves and save their bond.

    function attack(Claim _disputed, uint256 _parentIndex, Claim _claim) external payable {
        move(_disputed, _parentIndex, _claim, true);
    }

This is because for sub-gemes, when it comes time to distribute, if there is a counter, the bond is sent to the counter in resolveClaim

So a defender when know they are going to lose, there can just counter themselves

Proof of Concept

Tools Used

Manual

Recommended Mitigation Steps

Perhaps have a system where a log which way the game went. depending on the way the game went (for defenders or for challengers), A bond doesn't get forwarded to a counter unless the overall attack challenge won. And a bond doesn't get forwarded to a defend counter unless the over game won in terms of defense

Assessed type

Context

clabby commented 4 months ago

This report is valid. It is possible for a malicious challenger to post a well-formed output root hash as the rootClaim, and use their sole-knowledge of its preimage to be able to call challengeRootL2Block on themselves and steal the incentive from the honest challenge agent who would have challenged them by means of attack.

However, we currently don't plan on fixing this at the moment for several reasons:

  1. The incentive may only be seized at the top-depth of the dispute game.
  2. The malicious actor burns funds at a faster rate than the honest challenge agent (as they have to pay the gas for the proposal as well as the secondary counter via challengeRootL2Block, whereas the honest challenge agent only pays the gas for attack and receives their bond back.

In future iterations of the protocol, we will look to address this issue.

clabby commented 3 months ago

I misread this initially - but yes, this is still invalid imo. In the case where a dishonest party attacks their own claim, the challenger can also come in if it is a "freeloader" claim and threatens its incentive. Clock extension will also award extra time for the honest challenger to do so, if the malicious party ran down their clock first.

c4-judge commented 3 months ago

zobront marked the issue as unsatisfactory: Invalid