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

0 stars 0 forks source link

The LPP challenge period can cause malicious and freeloader claims to be uncounterable and can also cause freeloader claims to be abused to entrap honest challengers #29

Open c4-bot-9 opened 1 month ago

c4-bot-9 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-07-optimism/blob/main/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L371-L382

Vulnerability details

In some cases, the honest challenger must pass an LPP in order to counter a MAX_GAME_DEPTH claim through step(). For instance, "in the event that the preimage is too large to be submitted through calldata in a single block, challengers must resort to the streaming option" as described in the Optimism specs.

However, an issue arises because to pass an LPP, the honest challenger must wait for CHALLENGE_PERIOD seconds in order to pass the LPP to be able to step() against a malicious MAX_GAME_DEPTH claim. The honest challenger will have a minimum of one CLOCK_EXTENSION (3 hours on mainnet) period on their chess clock, which cannot cover the CHALLENGE_PERIOD for an LPP (1 day on mainnet).

There are essentially 3 impacts that this can cause:

Impact 1: A malicious MAX_GAME_DEPTH claim can be uncounterable if honest challenger does not have time left

If the honest challenger has less than 1 day left in their chess clock when reaching the malicious claim at MAX_GAME_DEPTH, they cannot possibly pass an LPP in time. This will cause the malicious claim to be uncountered while the honest challenger's claim will be countered leading to an incorrect game result.

Impact 2: Uncounterable freeloader claims can be made by forcing an honest challenger to inherit a clock smaller than CHALLENGE_PERIOD

The CLOCK_EXTENSION is a feature meant for honest challengers to counter a freeloader claim, a freeloader claim is described as follows:

Consider an attacker claim denoted by A in the subtree below. Suppose that this claim is valid, then the correct move for the honest party is to defend the claim. The honest party's claim is denoted by claim H.

   A  
 /   \
F     H

To prevent themselves from losing the bond, suppose the attacker then attacks claim A with the claim F. Due to the leftmost priority, they would receive the bond if their freeloader claim F remains uncountered. This is possible if the attacker's chess clock has very little time left because the honest party needs to use the attacker's chess clock to counter the freeloader claim.

To resolve this, Optimism uses clock extensions, where if a chess clock has less than CLOCK_EXTENSION seconds left, it is extended to CLOCK_EXTENSION seconds.

If the honest challenger must pass an LPP to counter the freeloader claim at MAX_GAME_DEPTH via step() it will not have enough time to do so, resulting in the freeloader claim stealing the bond from the honest party's claim.

Impact 3: Freeloader claims can now be used to bait honest challengers to respond and then entrap them, winning all the bonds they used to counter the freeloader claim

Combining the points raised in both impacts earlier, we can now see how freeloader claims can be used as bait to steal all the bonds used to counter the freeloader claim. The key point here is that when the honest challenger counters a freeloader claim, they essentially "swap" chess clocks with the attacker. All further moves made by the honest challenger now use the attacker's chess clock which can be manipulated to CLOCK_EXTENSION earlier. Now, let's take a look at the following scenario:

   A  
 /   \
F1    H1
|
H2
|
F2

Here, the attacker has responded to the honest user's claim H2, with their new claim F2. To counter F2, the honest challenger will still be using the attacker's chess clock which will be manipulated to CLOCK_EXTENSION time. Let's suppose F2 is a claim at MAX_GAME_DEPTH and to counter it the honest challenger must pass an LPP, as elaborated earlier they won't be able to do so and they essentially cannot counter F2. But now, they also will have their bonds for posting H2 stolen from them, as F2 will now counter H2. Furthermore, the freeloader claim F1 will still steal the bond from A using the leftmost priority.

Therefore, this technique can be used attackers to entrap honest challengers, by purposefully posting an invalid state that has a valid state preceding it that requires a max size LPP to execute step() on, they can further steal bonds away from the honest challengers to counter the freeloader claim.

Tools Used

Code review

Recommended Mitigation Steps

Apply a bigger extension such as CHALLENGE_PERIOD + CLOCK_EXTENSION at the MAX_GAME_DEPTH - 1 claim, as shown below.

+       IPreimageOracle oracle = VM.oracle();
+       if (nextPositionDepth == MAX_GAME_DEPTH - 1 && nextDuration.raw() > MAX_CLOCK_DURATION.raw() - oracle.challengePeriod() - CLOCK_EXTENSION.raw()) {
+           // Apply 1 CHALLENGE_PERIOD + 1 CLOCK_EXTENSION here. 
+           nextDuration = Duration.wrap(MAX_CLOCK_DURATION.raw() - oracle.challengePeriod() - CLOCK_EXTENSION.raw());
+       }
+       else if (nextDuration.raw() > MAX_CLOCK_DURATION.raw() - CLOCK_EXTENSION.raw()) {
-       if (nextDuration.raw() > MAX_CLOCK_DURATION.raw() - CLOCK_EXTENSION.raw()) {
            // If the potential grandchild is an execution trace bisection root, double the clock extension.
            uint64 extensionPeriod =
                nextPositionDepth == SPLIT_DEPTH - 1 ? CLOCK_EXTENSION.raw() * 2 : CLOCK_EXTENSION.raw();
            nextDuration = Duration.wrap(MAX_CLOCK_DURATION.raw() - extensionPeriod);
        }

This will solve all of the scenarios above.

Assessed type

Other

ajsutton commented 1 month ago

Impact 1 is invalid - the honest actor should be responding reasonably quickly and is expected to still have sufficient time on the clock to allow for the large preimage proposal time.

However the impact of this on freeloader claims and the clock extension being insufficient in the case of large preimages being required is valid and something we will need to fix.

zobront commented 1 month ago

This issue is valid, but given the clarification from the sponsor that Impact 1 is invalid, I believe it's likely that a Medium severity is more appropriate (ie worst case does not allow spoofing invalid state, only stealing some bonds).

I'll also need to consider whether the dups are valid as dups (or should receive partial credit), as this report was the most clear on the other implications.

I will consider more before finalizing decision.

c4-judge commented 1 month ago

zobront marked the issue as satisfactory

c4-judge commented 1 month ago

zobront marked the issue as selected for report

zobront commented 1 month ago

The other reports do identify the freeloader claims, which is the core issue. I will downgrade to Medium and leave the dups as-is receiving full credit.

c4-judge commented 1 month ago

zobront changed the severity to 2 (Med Risk)

Haxatron commented 1 month ago

Hi @zobront, we believe high is more appropriate for this finding. From impact 3 about entrapping the honest challenger, we note the loss of funds of the honest challenger which is very different from the loss of incentive. Here, "loss of funds" refers the funds lost by the honest challenger when posting their bonds and getting them stolen by the attacker and "loss of incentive" refers to the bonds entitled by the honest challenger for correctly countering the adversary claim.

Furthermore, the loss of fund impact is high, if we look at our example in impact 3, the honest challenger will post the bond H2 and lose their bond H2, which as MAX_GAME_DEPTH - 1 comes down to 270_000_000 * 200 gwei = 54 ETH which is by no means a small amount. This loss is also cumulative, as the honest challenger will lose all their bonds they posted to counter the freeloader claim all the way down to MAX_GAME_DEPTH.

All in all, the "loss of funds" impact to the honest challenger is high, so we also believe this should be a high.

Haxatron commented 1 month ago

https://github.com/code-423n4/2024-07-optimism-findings/issues/122 talks about MAX_CLOCK_DURATION and not CLOCK_EXTENSION, which is in no way related to this finding so it is not a dupe.

0xEVom commented 1 month ago

@zobront report #26, on the other hand, only points out impact 2 in this finding. The main difference between impacts 2 and 3 is:

Because #26 only points out impact 2 and not the higher impact 3, partial credit would seem fair.

c4-judge commented 1 month ago

zobront changed the severity to QA (Quality Assurance)

c4-judge commented 1 month ago

This previously downgraded issue has been upgraded by zobront

zobront commented 1 month ago

After much consideration, I'm in agreement with @Haxatron and @0xEVom here on all counts.

1) For consistency in judging, the other issues deemed High either allowed the VM to act in a way that would allow incorrect output roots to be proven, or would allow valid claims to be challenged in a predictable way to allow intentionally stealing a large amount of bonds. This falls into the latter category, so will be upgraded to High.

2) #26 does capture the root cause here, and gets most of the way there, but on its own would be judged as a Medium because it doesn't show how bonds could be stolen. I will be moving to partial credit, but because so much of the exploit was understood, I will award 75%.

3) Agreed that the QA report that I upgraded does not capture the finding, so I am moving back down to QA.

Thank you for your thoughtful analysis here.

c4-judge commented 1 month ago

zobront changed the severity to 3 (High Risk)