sherlock-audit / 2024-03-nouns-dao-2-judging

1 stars 0 forks source link

ether_sky - An important proposal can expire during the forking period. #26

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

ether_sky

medium

An important proposal can expire during the forking period.

Summary

Succeeded proposals move to the timelock for execution. During the grace period, they can only be executed. After this grace period, proposals expire. And during the forking period, no proposals can be executed. Some important proposals might expire due to the overlapping grace period and forking period. Then we need to recreate the same proposal and gather votes again. However, this process takes time, and there is no guarantee they will pass again.

Vulnerability Detail

We can only execute the proposals during the grace period in the timelock.

function executeTransaction(
    address target,
    uint256 value,
    string memory signature,
    bytes memory data,
    uint256 eta
) public returns (bytes memory) {
    require(
        getBlockTimestamp() >= eta,
        "NounsDAOExecutor::executeTransaction: Transaction hasn't surpassed time lock."
    );
    require(
        getBlockTimestamp() <= eta + GRACE_PERIOD,
        'NounsDAOExecutor::executeTransaction: Transaction is stale.'
    );
}

During the forking period, executing proposals is not possible.

function executeInternal(
    NounsDAOTypes.Storage storage ds,
    NounsDAOTypes.Proposal storage proposal,
    INounsDAOExecutor timelock
) internal {
    if (ds.isForkPeriodActive()) revert CannotExecuteDuringForkingPeriod();
}

As a result, some proposals may expire even though they are eligible for execution.

Impact

Code Snippet

https://github.com/sherlock-audit/2024-03-nouns-dao-2/blob/8f6879efaf831eb7fc9d4a4ad2b62b5334220d87/nouns-monorepo/packages/nouns-contracts/contracts/governance/NounsDAOExecutorV2.sol#L178-L185 https://github.com/sherlock-audit/2024-03-nouns-dao-2/blob/8f6879efaf831eb7fc9d4a4ad2b62b5334220d87/nouns-monorepo/packages/nouns-contracts/contracts/governance/NounsDAOProposals.sol#L452

Tool used

Manual Review

Recommendation

If the execute proposal call is invoked during the forking period, we can extend the eta of it to the end of the forking period.

WangSecurity commented 4 months ago

I believe it's intended design and clearly see your point, but as I understand forking will be know a lot of time before it, so the users can be careful with their proposals.