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

6 stars 4 forks source link

MiloTruck - `DisputeGameFactory.create()` does not protect against L1 re-orgs #152

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

MiloTruck

medium

DisputeGameFactory.create() does not protect against L1 re-orgs

Summary

An L1 re-org could cause a previously valid output root to become invalid, causing honest proposers to lose their bonds for creating dispute games.

Vulnerability Detail

Under the documentation of L2OutputOracle proposals, it mentions that L1 re-orgs might invalidate the output root being proposed:

Security Considerations

L1 reorgs

If the L1 has a reorg after an output has been generated and submitted, the L2 state and correct output may change leading to a faulty proposal. This is mitigated against by allowing the proposer to submit an L1 block number and hash to the Output Oracle when appending a new output; in the event of a reorg, the block hash will not match that of the block with that number and the call will revert.

This means that the following could occur:

L2OutputOracle includes the following check to protect against such a scenario:

L2OutputOracle.sol#L211-L224

        if (_l1BlockHash != bytes32(0)) {
            // This check allows the proposer to propose an output based on a given L1 block,
            // without fear that it will be reorged out.
            // It will also revert if the blockheight provided is more than 256 blocks behind the
            // chain tip (as the hash will return as zero). This does open the door to a griefing
            // attack in which the proposer's submission is censored until the block is no longer
            // retrievable, if the proposer is experiencing this attack it can simply leave out the
            // blockhash value, and delay submission until it is confident that the L1 block is
            // finalized.
            require(
                blockhash(_l1BlockNumber) == _l1BlockHash,
                "L2OutputOracle: block hash does not match the hash at the expected height"
            );
        }

However, the new DisputeGameFactory.create() function, which is meant to replace L2OutputOracle's functionality, no longer has this check, leaving it vulnerable to the scenario mentioned above.

Impact

When an honest proposer calls create() with a legitimate output root, an L1 re-org could cause the output root to become invalid. This would cause the proposer to lose his bond for creating the dispute game as his proposed output root is now invalid after the L1 reorg.

Code Snippet

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

Tool used

Manual Review

Recommendation

Similar to L2OutputOracle, consider adding the L1 block number and hash as parameters to DisputeGameFactory.create(), and checking if the blockhash of the current block matches the proposer's input.

nevillehuang commented 7 months ago

Invalid based on sherlock rules

  1. Chain re-org and network liveness related issues are not considered valid.