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

1 stars 1 forks source link

bareli - StakingRewards: Significant loss of precision possible #55

Closed sherlock-admin2 closed 1 month ago

sherlock-admin2 commented 1 month ago

bareli

Medium

StakingRewards: Significant loss of precision possible

Summary

In notifyRewardAmount, the reward rate per second is calculated. This calculation rounds down, which can lead to situations where significantly less rewards are paid out to stakers, because the effect of the rounding is multiplied by the duration.

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

et's say we have a rewardsDuration of 4 years, i.e. 126144000 seconds. We assume the rewardRate is currently ß and notifyRewardAmount is called with the reward amount 252287999. Because the calculation rounds down, rewardRate will be 1. After the 4 years, the user have received 126144000 reward tokens. However, 126143999 (i.e., almost 50%) of the reward tokens that were intended to be distributed to the stakers were not distributed, resulting in monetary loss for all stakers. function notifyRewardAmount(uint256 _reward, uint256 _rewardUsdc)

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

You could accumulate the differences that occur due to rounding and let the users claim them in the end according to their shares.

Duplicate of #3

telome commented 1 month ago

Known issue. See Section 5.1 of https://github.com/makerdao/endgame-toolkit/blob/5dc625fd6a07c7c24a97a45553c2287f38807e44/audits/ChainSecurity_Maker_EndGame_Toolkit_audit_v3.pdf