code-423n4 / 2023-05-maia-findings

20 stars 12 forks source link

Time constants in `GovernorBravoDelegateMaia.sol` are wrong #728

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/governance/GovernorBravoDelegateMaia.sol#L17-L27

Vulnerability details

Impact

In GovernorBravoDelegateMaia.sol contract at the beginning we have several constants. We should pay more attention to the following:

    /// @notice The minimum setable voting period
    uint256 public constant MIN_VOTING_PERIOD = 80640; // About 2 weeks  

    /// @notice The max setable voting period
    uint256 public constant MAX_VOTING_PERIOD = 161280; // About 4 weeks

    /// @notice The min setable voting delay
    uint256 public constant MIN_VOTING_DELAY = 40320; // About 1 weeks

    /// @notice The max setable voting delay
    uint256 public constant MAX_VOTING_DELAY = 80640; // About 2 weeks

These constants define the minimum and maximum constants for the voting period and voting delay.

As we can see from the placed comments it is assumed that they are for this period of time because the developers calculate that a block in the Ethereum network is minted in 15 seconds. But not only is this not true (the average time is 12 sec in mainnet), but different chains are minted at different times. Possibly even for 1 second.

Proof of Concept

From the documentation we can see:

  • Is it multi-chain?: true

The average block time in Ethereum is 12s. This means 1 block every 12 sec is 5 blocks. The duration for 1 week is `5 60 24 = 50400.

But this is not the only problem. MaiaDao is multi-chain. I can give the following examples to see how wrong it is to hardcode time constants:

These constants are used throughout the contract for checks that will lead to totally wrong results and expectations. It's important to consider the specific block time of the blockchain where the smart contract is deployed and adjust each value accordingly to achieve the desired frequency.

Tools Used

Visual Studio Code

Recommended Mitigation Steps

Instead of hardcoding these constants, the best idea is to set them in the initialize() for each chain. By setting the duration dynamically in the initialize(), you ensure that the contract adapts to the block time of each network, making the duration consistent and appropriate for each specific blockchain environment.

Assessed type

Timing

c4-judge commented 1 year ago

trust1995 marked the issue as primary issue

c4-judge commented 1 year ago

trust1995 marked the issue as satisfactory

c4-sponsor commented 1 year ago

0xLightt marked the issue as sponsor confirmed

c4-judge commented 1 year ago

trust1995 marked issue #417 as primary and marked this issue as a duplicate of 417