peanuts - It is not possible to reprove a withdrawal hash if CHALLENGER_WINS

[sherlock-admin3]

peanuts

medium

It is not possible to reprove a withdrawal hash if CHALLENGER_WINS

Summary

If CHALLENGER_WINS, user cannot reprove a withdrawal.

Vulnerability Detail

When proving a withdrawal, the user can reprove a withdrawal if either of these 4 conditions exist:

        require(
            provenWithdrawal.timestamp == 0 || oldGame.status() == GameStatus.CHALLENGER_WINS
                || disputeGameBlacklist[oldGame] || oldGame.gameType().raw() != respectedGameType.raw(),
            "OptimismPortal: withdrawal hash has already been proven, and the old dispute game is not invalid"
        );

1. The withdrawal is not proven yet
2. The status resolves to CHALLENGER_WINS
3. The disputeGame is in blacklist
4. The gameType has changed.

However, oldGame.status() == GameStatus.CHALLENGER_WINS cannot be possible because of the above check that the status cannot be CHALLENGER_WINS.

  // We do not allow for proving withdrawals against dispute games that have resolved against the favor
        // of the root claim.
        require(
>           gameProxy.status() != GameStatus.CHALLENGER_WINS,
            "OptimismPortal: cannot prove against invalid dispute games"
        );

        IDisputeGame oldGame = provenWithdrawal.disputeGameProxy;
        require(
            provenWithdrawal.timestamp == 0 || oldGame.status() == GameStatus.CHALLENGER_WINS
                || disputeGameBlacklist[oldGame] || oldGame.gameType().raw() != respectedGameType.raw(),
            "OptimismPortal: withdrawal hash has already been proven, and the old dispute game is not invalid"
        );

When users prove a transaction, the oldGame can never be CHALLENGER_WINS because the status of the gameProxy cannot be CHALLENGER_WINS.

        provenWithdrawals[withdrawalHash][msg.sender] =
            ProvenWithdrawal({ disputeGameProxy: gameProxy, timestamp: uint64(block.timestamp) });

Impact

Users cannot reprove their withdrawal has if CHALLENGER_WINS.

Code Snippet

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

Tool used

Manual Review

Recommendation

Remove this require status that checks whether the game is not CHALLENGER_WINS (It does not really matter because in finalize withdrawal, the game must resolve to DEFENDER_WINS).

require(
            gameProxy.status() != GameStatus.CHALLENGER_WINS,
            "OptimismPortal: cannot prove against invalid dispute games"
        );

[nevillehuang]

Invalid as per noted in comments here