Rewards for initial period can be lost in contracts

Summary

This function calculates the reward rate per second and also records the start of the reward period. This has an edge case where rewards are not counted for the initial period of time until there is at least one participant.

Vulnerability Detail

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; }

    // Ensure the provided reward amount is not more than the balance in the contract.
    // This keeps the reward rate in the right range, preventing overflows due to
    // very high values of rewardRate in the earned and rewardsPerToken functions;
    // Reward + leftover must be less than 2^256 / 10^18 to avoid overflow.
    uint256 balance = rewardsToken.balanceOf(address(this));
    require(rewardRate <= balance / rewardsDuration, "Provided reward too high");

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

Impact

The intention here, is to calculate how many tokens should be rewarded by unit of time (second) and record the span of time for the reward cycle. However, this has an edge case where rewards are not counted for the initial period of time until there is at least one participant (in this case, a holder of BathTokens). During this initial period of time, the reward rate will still apply but as there isn't any participant, then no one will be able to claim these rewards and these rewards will be lost and stuck in the system.

This is a known vulnerability that has been covered before. The following reports can be used as a reference for the described issue:

(https://0xmacro.com/blog/synthetix-staking-rewards-issue-inefficient-reward-distribution/) https://github.com/code-423n4/2022-09-y2k-finance-findings/issues/93 As described by the 0xmacro blogpost, this can play out as the following:

Let's consider that you have a StakingRewards contract with a reward duration of one month seconds (2592000):

Block N Timestamp = X

You call notifyRewardAmount() with a reward of one month seconds (2592000) only. The intention is for a period of a month, 1 reward token per second should be distributed to stakers.

State :

rewardRate = 1 periodFinish = X + 2592000 Block M Timestamp = X + Y

Y time has passed and the first staker stakes some amount:

stake() updateReward rewardPerTokenStored = 0 lastUpdateTime = X + Y Hence, for this staker, the clock has started from X+Y, and they will accumulate rewards from this point.

Please note, that the periodFinish is X + rewardsDuration, not X + Y + rewardsDuration. Therefore, the contract will only distribute rewards until X + rewardsDuration, losing Y rewardRate => Y 1 inside of the contract, as rewardRate = 1 (if we consider the above example).

Now, if we consider delay (Y) to be 30 minutes, then:

Only 2590200 (2592000-1800) tokens will be distributed and these 1800 tokens will remain unused in the contract until the next cycle of notifyRewardAmount().

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/synthetix/StakingRewards.sol#L144C4-L163C6

Tool used

Manual Review

Recommendation

possible solution to the issue would be to set the start and end time for the current reward cycle when the first participant joins the reward program (i.e. when the total supply is greater than zero) instead of starting the process in the notifyRewardAmount.

Duplicate of #96

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

bareli - Rewards for initial period can be lost in contracts #57