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

6 stars 4 forks source link

Trust - Incorrect enforcement of bottom-up resolution leads to unresolvable claims #202

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

Trust

high

Incorrect enforcement of bottom-up resolution leads to unresolvable claims

Summary

Incorrect enforcement of bottom-up resolution leads to unresolvable claims.

Vulnerability Detail

When the dispute game ends, claims are resolved from the bottom-up, to ensure challenges are properly settled. When resolving a claim, it is verified that no challenger of that claim has it's own challenges. Otherwise, we view any non-countered challenges of the current claim as legitimate.

for (uint256 i = 0; i < challengeIndicesLen; ++i) {
    uint256 challengeIndex = challengeIndices[i];
    // INVARIANT: Cannot resolve a subgame containing an unresolved claim
    if (subgames[challengeIndex].length != 0) revert OutOfOrderResolution();
    ClaimData storage claim = claimData[challengeIndex];
    // If the child subgame is uncountered and further left than the current left-most counter,
    // update the parent subgame's `countered` address and the current `leftmostCounter`.
    // The left-most correct counter is preferred in bond payouts in order to discourage attackers
    // from countering invalid subgame roots via an invalid defense position. As such positions
    // cannot be correctly countered.
    // Note that correctly positioned defense, but invalid claimes can still be successfully countered.
    if (claim.counteredBy == address(0) && leftmostCounter.raw() > claim.position.raw()) {
        countered = claim.claimant;
        leftmostCounter = claim.position;
    }
}

Another requirement in order to resolve claims is that the game must still be in progress: if (status != GameStatus.IN_PROGRESS) revert GameNotInProgress();

An issue occurs because although the game resolution logic is sound in this case, the state management allows a situation where a claim that has not been resolved yet will never be resolvable. Consider the situation below:

Claim B -> Claim A

Claim A is the root claim and claim B counters it. When the game timer ends for A, it can be resolved (there are no subgames for claim B). However, since A is the root, resolve() can now be called, which changes the status of the game to CHALLENGER_WINS. However, now resolveClaim() can never be called for B (the in progress check will fail).

As a result, the bond for claim B cannot be claimed, leading to loss of funds of the honest claimer.

Impact

An honest participant may lose their bond.

Code Snippet

Tool used

Manual Review

Recommendation

Two options:

Duplicate of #164

sherlock-admin2 commented 6 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/ethereum-optimism/optimism/pull/10148