datapunk - The special case of “_epochBegin == block.timestamp” is undefined

[sherlock-admin]

datapunk

high

The special case of “_epochBegin == block.timestamp” is undefined

Summary

The special case of “_epochBegin == block.timestamp” is undefined, such that both epochHasNotStarted and epochHasStarted modifiers will allow further operations

Vulnerability Detail

Since == is not taken into account, when comparing block.timestamp and epochConfig[_id].epochBegin, the system may get into weird states. For example, deposit() may happen after resolveEpoch() is called in the same block, thus ending up with inaccurate finalTVL[_id].

    modifier epochHasNotStarted(uint256 _id) {
        if (block.timestamp > epochConfig[_id].epochBegin)
            revert EpochAlreadyStarted();
        _;
    }

    /** @notice You can only call functions that use this modifier after the epoch has started
     */
    modifier epochHasStarted(uint256 _id) {
        if (block.timestamp < epochConfig[_id].epochBegin)
            revert EpochNotStarted();
        _;
    }

Impact

finalTVL[_id] may be set before deposit(), thus does not represent the actual full amount. Since a lot of calculations such as calculateWithdrawalFeeValue, previewEmissionsWithdraw depend on finalTVL[_id], those calculated results will be incorrect.

Code Snippet

https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L432-L444 https://github.com/sherlock-audit/2023-03-Y2K/blob/main/Earthquake/src/v2/VaultV2.sol#L93

Tool used

Manual Review

Recommendation

Change to


modifier epochHasNotStarted(uint256 _id) {
        if (block.timestamp >= epochConfig[_id].epochBegin)
            revert EpochAlreadyStarted();
        _;
    }

Duplicate of #480