PrecisionLoss in NotifyRewardAmount

Summary

The notifyRewardAmount function in the smart contract has an incorrect validation mechanism for the reward rate. The current implementation uses integer division to check if the reward rate is within the contract's balance, leading to potential precision loss and incorrect validation, especially over long durations like 7 days. This can cause the function to pass invalid reward rates, resulting in incorrect reward distributions. https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/synthetix/StakingRewards.sol#L144

Vulnerability Detail

The require statement intended to validate the reward rate uses integer division, which truncates the result, leading to potential precision loss. This can result in an incorrect validation of the reward rate, particularly in scenarios where the division leads to unexpected results due to truncation.

rewardsDuration = 7 days = 604800 seconds balance = 1000000 tokens rewardRate initially set to 0

Let's say we want to set reward = 500000 tokens.

rewardRate = reward / rewardsDuration; // 500000 / 604800 ≈ 0.826 tokens per second

balance / rewardsDuration = 1000000 / 604800 ≈ 1.653 tokens per second (integer division truncates to 1)

rewardRate ≈ 0.826 balance / rewardsDuration ≈ 1

Since 0.826 <= 1, the require statement passes.

Let assume a scenario where the balance is less than the RewardDuration due to reduction of balance and of that. Assume balance = 10000 tokens and rewardsDuration = 7 days = 604800 seconds.

rewardRate = reward / rewardsDuration; // 4 / 604800 ≈ 0 (since 4 < 604800)

balance / rewardsDuration = 10 / 604800 ≈ 0 (integer division truncates to 0)

rewardRate = 0 balance / rewardsDuration = 0

Since 0 <= 0, the require statement passes. However, let's look at what happens in terms of rewards distribution:

Total distributed reward: rewardRate rewardsDuration = 0 604800 = 0 tokens. Given reward: 4 tokens.

Impact

Precision loss in the balance of tokens to be distributed to the stakers, resulting in incorrect reward distributions.

Code Snippet

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

Tool used

Manual Review

Recommendation

You could accumulate the differences that occur due to precision/truncation and let users claim them at the end and according to their shares

Duplicate of #3

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

EFCCWEB3 - PrecisionLoss in NotifyRewardAmount #2