reentrancy attack may happen in "initialize()"

Summary

when calling the initialize function we are initially keeping initialized variable as false. Then we are calling the
" WETH.deposit{ value: msg.value }();" function but if WETH contract is not trusted or can cause calling of initialize function again ,this may cause a reentrancy as our initialized is false till now.

Vulnerability Detail

function initialize() public payable virtual { // SAFETY: Any revert in this function will bubble up to the DisputeGameFactory and // prevent the game from being created. // // Implicit assumptions: // - The gameStatus state variable defaults to 0, which is GameStatus.IN_PROGRESS // - The dispute game factory will enforce the required bond to initialize the game. // // Explicit checks: // - The game must not have already been initialized. // - An output root cannot be proposed at or before the starting block number.

    // INVARIANT: The game must not have already been initialized.
    if (initialized) revert AlreadyInitialized();

    // 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 });

    // Do not allow the game to be initialized if the root claim corresponds to a block at or before the
    // configured starting block number.
    if (l2BlockNumber() <= rootBlockNumber) revert UnexpectedRootClaim(rootClaim());

    // Revert if the calldata size is too large, which signals that the `extraData` contains more than expected.
    // This is to prevent adding extra bytes to the `extraData` that result in a different game UUID in the factory,
    // but are not used by the game, which would allow for multiple dispute games for the same output proposal to
    // be created.
    // Expected length: 0x66 (0x04 selector + 0x20 root claim + 0x20 l1 head + 0x20 extraData + 0x02 CWIA bytes)
    assembly {
        if gt(calldatasize(), 0x66) {
            // Store the selector for `ExtraDataTooLong()` & revert
            mstore(0x00, 0xc407e025)
            revert(0x1C, 0x04)
        }
    }

    // Set the root claim
    claimData.push(
        ClaimData({
            parentIndex: type(uint32).max,
            counteredBy: address(0),
            claimant: tx.origin,
            bond: uint128(msg.value),
            claim: rootClaim(),
            position: ROOT_POSITION,
            clock: LibClock.wrap(Duration.wrap(0), Timestamp.wrap(uint64(block.timestamp)))
        })
    );

    // Deposit the bond.

@>> WETH.deposit{ value: msg.value }();

    // Set the game's starting timestamp
    createdAt = Timestamp.wrap(uint64(block.timestamp));

    // Set the game as initialized.

@>> initialized = true; }

Impact

reentrancy attack may happen in "initialize()".

Code Snippet

https://github.com/sherlock-audit/2024-02-optimism-2024/blob/main/optimism/packages/contracts-bedrock/src/dispute/FaultDisputeGame.sol#L513

Tool used

Manual Review

Recommendation

// Deposit the bond.

@>> initialized = true; @>> WETH.deposit{ value: msg.value }();

    // Set the game's starting timestamp
    createdAt = Timestamp.wrap(uint64(block.timestamp));

    // Set the game as initialized.

}

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

bareli - reentrancy attack may happen in "initialize()" #225