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

6 stars 4 forks source link

nirohgo - Blacklisting a game does not remove it from the AnchorStateRegistry, forcing subsequent games to use it as the starting Claim which disrupts their correct resolution. #207

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 7 months ago

nirohgo

high

Blacklisting a game does not remove it from the AnchorStateRegistry, forcing subsequent games to use it as the starting Claim which disrupts their correct resolution.

Summary

A game that resolves in favor of the defender and is set in AnchorStateRegistry, then later discovered as faulty and blacklisted, is not removed from the AnchorStateRegistry, causing subsequent games to use is as the stating claim which prevents them from resolving correctly.

Vulnerability Detail

Blacklisting a game has only one effect, which is to prevent finalizing a withdrawal that was proved based on it:

 function checkWithdrawal(bytes32 _withdrawalHash, address _proofSubmitter) public view {
ProvenWithdrawal memory provenWithdrawal = provenWithdrawals[_withdrawalHash][_proofSubmitter];
IDisputeGame disputeGameProxy = provenWithdrawal.disputeGameProxy;

// The dispute game must not be blacklisted.
require(!disputeGameBlacklist[disputeGameProxy], "OptimismPortal: dispute game has been blacklisted");

However, if the game has already been resolved in favor of the defender, it is stored in AnchorStateRegistry as the latest proven root for that game type. There is no way to remove it from the anchor list except for resolving a later game that will replace it. But any subsequent game that will be created, will use the root of the blacklisted game stored in AnchorStateRegisty as its starting root (FaultDisputeGame.sol line 528):

// Grab the latest anchor root.
(Hash root, uint256 rootBlockNumber) = ANCHOR_STATE_REGISTRY.anchors(GAME_TYPE);

// Should only happen if this is a new game type that hasn't been set up yet.
if (root.raw() == bytes32(0)) revert AnchorRootNotFound();

// Set the starting output root.
startingOutputRoot = OutputRoot({ l2BlockNumber: rootBlockNumber, root: root });

This means that the blacklisted game root will be the starting Claim of the game (based on the assumption that it is the last known L2 state to be proven correct) in spite of being corrupt. As a result, subsequent games will have to be blacklisted as well, forcing a full system restart including all the implications.

vulnerability scenario:

  1. a game is created to prove L2 block X.
  2. The game is resolved correctly in favor of the defender.
  3. Subsequent monitoring discovers that the game was faulty and that its root claim does not represent the real L2 state at block X.
  4. The Guardian marks the game as blacklisted before DISPUTE_GAME_FINALITY_DELAY_SECONDS preventing any withdrawals that have used it to be finalized.
  5. The game however has already been set as the latest Anchor is AnchorStateRegistry and can only be replaced by a subsequent game that resolves correctly.
  6. However, every subsequent game will have to use the faulty game as its starting claim (because it is the anchor), preventing correct resolution.
  7. The only way to restore the system to functioning state is through a complex upgrade (had the blacklisted game been correctly removed from ANCHOR_STATE_REGISTRY, a new valid game proving X would have replaced it at the anchor allowing the system to continue to function without the need for a rehaul).

Impact

The vulnerability causes an unnessecary DOS of the system for the duration of the required upgrade to resolve the problem, estimated at well over a week. In addition, subsequent games based on the corrupt starting point may actually resolve succesfully inspite of proving false roots themselves, enabling rogue withdrawals. System monitoring may not detect the problem in those subsequent games, allowing the false withdrawals to be finalized.

Code Snippet

https://github.com/sherlock-audit/2024-02-optimism-2024/blob/f216b0d3ad08c1a0ead557ea74691aaefd5fd489/optimism/packages/contracts-bedrock/src/L1/OptimismPortal2.sol#L440

Tool used

Manual Review
Foundry

Recommendation

in OptimismPortal2::blacklistDisputeGame, when a game is blacklisted, make sure to remove it from the ANCHOR_STATE_REGISTRY if it had already been set there.

Duplicate of #81