Game creator can create games whose L2 block cannot be challenged even when it contains absurd values

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

Malicious Game creator can cause challengeRootL2Block to revert, which would prevent challenging the L2 block of games with obviously invalid values.

Proof of Concept

Have a look at FaultDisputeGame#challengeRootL2Block function:

function challengeRootL2Block(
    Types.OutputRootProof calldata _outputRootProof,
    bytes calldata _headerRLP
)
    external
{
    // INVARIANT: Moves cannot be made unless the game is currently in progress.
    if (status != GameStatus.IN_PROGRESS) revert GameNotInProgress();

    // The root L2 block claim can only be challenged once.
    if (l2BlockNumberChallenged) revert L2BlockNumberChallenged();

    // Verify the output root preimage.
    if (Hashing.hashOutputRootProof(_outputRootProof) != rootClaim().raw()) revert InvalidOutputRootProof();

    // Verify the block hash preimage.
    if (keccak256(_headerRLP) != _outputRootProof.latestBlockhash) revert InvalidHeaderRLP();

    // Decode the header RLP to find the number of the block. In the consensus encoding, the timestamp
    // is the 9th element in the list that represents the block header.
    RLPReader.RLPItem[] memory headerContents = RLPReader.readList(RLPReader.toRLPItem(_headerRLP));
    bytes memory rawBlockNumber = RLPReader.readBytes(headerContents[HEADER_BLOCK_NUMBER_INDEX]);

    // Sanity check the block number string length.
+++ if (rawBlockNumber.length > 32) revert InvalidHeaderRLP();
    ...
}

The function reverts if rawBlockNumber.length>32. This allows the game creator to create games, inputting rootClaims whose block number cannot be challenged.

rootClaim is an hash of _outputRootProof _outputRootProof.latestBlockHash is an hash of _headerRLP _headerRLP is parsed into an array, then read to get the rawBlockNumber(which is at the 8th position in the array)

When constructing the root claim, malicious user can

• set the rawBlockNumber to a >32 bytes value, store it in the _headerRLP
• hash the _headerRLP to give the _outputRootProof.latestBlockHash
• hashOutputRootProof the _outputRootProof to get the rootClaim.
• create a FDG with the rootClaim
• Now, noone will be able to challengeRootL2Block, even though the rawBlockNumber contained in it is an absurd value

Tools Used

Manual Review

Recommended Mitigation Steps

The sanity check for the block number string length should be done during game creation, not in the challengeRootL2Block function.

Assessed type

Invalid Validation

[ajsutton]

This is invalid because, the op-program execution in the VM will prove that the posted output root is invalid (valid output roots never have rawBlockNumber.length > 32). The derivation process it runs will stop when the L1 head is reached and the claimed output root does not match the current safe head. So the honest actor can just play out the dispute game to invalidate the proposal rather than calling challengeRootL2Block. The challengeRootL2Block function is only required to invalidate outputs in the case where the valid output root of the safe head is posted but with a future L2Block number claimed.

[c4-judge]

zobront marked the issue as unsatisfactory: Invalid