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

6 stars 4 forks source link

FonDevs - Incorrect check if the calldatasize is too large when creating a new `GameType` #189

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 7 months ago

FonDevs

high

Incorrect check if the calldatasize is too large when creating a new GameType

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

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

Summary

the check for the calldatasize to prevent it from being too large is done in the FaultDisputeGame function initialize instead of the DisputeGameFactory function create. this allows multiple dispute games for the same output proposal to be created.

Vulnerability Detail

the solidity opcode calldatasize returns the size of the calldata in the current environment.


assembly {

            // change the check from 0x66 to 0x

            if gt(calldatasize(), 0x66) {
                // Store the selector for `ExtraDataTooLong()` & revert
                mstore(0x00, 0xc407e025)
                revert(0x1C, 0x04)
            }
        }

so when the above check is done in the FaultDisputeGame function initialize instead of the DisputeGameFactory function create since the calldatasize in the function initialize is constant, large bytes calldata _extraData can be used to create multiple dispute games because the hash will be different for different _extraData arguments.

Impact

this allows multiple dispute games for the same output proposal to be created.

Code Snippet


// SPDX-License-Identifier: MIT
pragma solidity ^0.8.15;

contract kinstar {
    uint256 public size;

    function second(uint256, uint256) external {
        uint256 calldt;
        assembly {
            calldt := calldatasize()
        }
        size = calldt;
    }
}

contract baby {
    kinstar kin;
    uint256 public size;

    function first(uint256 wavy, uint256 ruger, uint256 ) external {
        uint256 calldt;
        kin.second(wavy, ruger);
        assembly {
            calldt := calldatasize()
        }
        size = calldt;
    }
}

Tool used

Manual Review

Recommendation

the code block to check the calldatasize should be done in the function create in the DisputeGameFactory instead of the function initialize in the FaultDisputeGame.

the check for the expected calldatasize should be equal to ---

Duplicate of #203

sherlock-admin2 commented 5 months ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/ethereum-optimism/optimism/pull/10208