The stateInternal function mistakenly identifies proposals as expired when the grace period concludes.

Summary

With in the timelock, succeeded proposals can be executed during the grace period. They can be executed when the grace period concludes. However, the stateInternal function identifies proposals as expired at this time. As a result, users might inadvertently skip executing them.

Vulnerability Detail

We can execute proposals when the grace period concludes.

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,  // @audit, here
        'NounsDAOExecutor::executeTransaction: Transaction is stale.'
    );
}

However, the stateInternal function identifies proposals as expired at this time.

function stateInternal(
    NounsDAOTypes.Storage storage ds,
    uint256 proposalId
) internal view returns (NounsDAOTypes.ProposalState) {
    else if (block.timestamp >= proposal.eta + getProposalTimelock(ds, proposal).GRACE_PERIOD()) {  // @audit, here
        return NounsDAOTypes.ProposalState.Expired;
    }
}

The proposals can not be executed when the grace period concludes, or we modify the stateInternal function.

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#L607-L609

Tool used

Manual Review

Recommendation

function stateInternal(
    NounsDAOTypes.Storage storage ds,
    uint256 proposalId
) internal view returns (NounsDAOTypes.ProposalState) {
-    else if (block.timestamp >= proposal.eta + getProposalTimelock(ds, proposal).GRACE_PERIOD()) {
+    else if (block.timestamp > proposal.eta + getProposalTimelock(ds, proposal).GRACE_PERIOD()) {
        return NounsDAOTypes.ProposalState.Expired;
    }
}

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

ether_sky - The stateInternal function mistakenly identifies proposals as expired when the grace period concludes. #28