Permissionless distribute() function can extend the period finish time

Summary

Anyone could extend the reward finish time by calling permissionless distribute() function from VestedRewardsDistribution.sol, potentially resulting in users receiving fewer rewards than expected within the same time period.

Vulnerability Detail

The only requirement for calling this function is if the amount is greater than 0:

function distribute() external returns (uint256 amount) {
        require(vestId != INVALID_VEST_ID, "VestedRewardsDistribution/invalid-vest-id");

        amount = dssVest.unpaid(vestId);
>>>     require(amount > 0, "VestedRewardsDistribution/no-pending-amount");

        lastDistributedAt = block.timestamp;
        dssVest.vest(vestId, amount);

        require(gem.transfer(address(stakingRewards), amount), "VestedRewardsDistribution/transfer-failed");
>>>     stakingRewards.notifyRewardAmount(amount);

        emit Distribute(amount);
    }

function notifyRewardAmount(uint256 reward) external override onlyRewardsDistribution updateReward(address(0)) {
        if (block.timestamp >= periodFinish) {
            rewardRate = reward / rewardsDuration;
        } else {
            uint256 remaining = periodFinish - block.timestamp;
            uint256 leftover = remaining * rewardRate;
            rewardRate = (reward + leftover) / rewardsDuration;
        }

        uint256 balance = rewardsToken.balanceOf(address(this));
        require(rewardRate <= balance / rewardsDuration, "Provided reward too high");

        lastUpdateTime = block.timestamp;
>>>     periodFinish = block.timestamp + rewardsDuration;
        emit RewardAdded(reward);
    }

Variable amount is calculated based on timestamp and some other variables:

 /**
        @dev amount of tokens accrued, not accounting for tokens paid
        @param _time The timestamp to perform the calculation
        @param _bgn  The start time of the contract
        @param _clf  The timestamp of the cliff
        @param _fin  The end time of the contract
        @param _tot  The total amount of the contract
        @param _rxd  The number of gems received
        @return amt  The claimable amount
    */
    function unpaid(uint256 _time, uint48 _bgn, uint48 _clf, uint48 _fin, uint128 _tot, uint128 _rxd) internal pure returns (uint256 amt) {
        amt = _time < _clf ? 0 : sub(accrued(_time, _bgn, _fin, _tot), _rxd);
    }

It is possible for a malicious user to calculate when this function will return very small values, making the attack almost cost-free. Calling this function will break calculations: extend the period finish time and decrease reward rate. This could result in loss of rewards: if there are 10 DAI rewards within a 10-day period, a malicious user could extend the finish time on day 5, extending the finish time to the 15th day. Participants would only receive 7.5 DAI by the 10th day.

Impact

Anyonce could extend the reward finish time and the users may receive less rewards than expected during the same time period.

Code Snippet

Link 1 Link 2

Tool used

Manual Review

Recommendation

Make the function permissioned:

-   function distribute() external returns (uint256 amount)
+   function distribute() external returns (uint256 amount) auth

sherlock-audit / 2024-06-makerdao-endgame-judging

DenTonylifer - Permissionless distribute() function can extend the period finish time #123