Rewards are distributed even when there are no stakers, resulting in the rewards being permanently locked away

Summary

An issue causes the system to mistakenly believe that rewards are being dispersed even in the absence of stakers. If notifyRewardAmount() is called before any users stake, the rewards intended for the first stakers become permanently locked in the contract. This issue results in non-distributed rewards being stuck in the contract.

Vulnerability Detail

The code accounts for scenarios where there are no users by not updating the cumulative rate when _totalSupply is zero. However, it fails to include a similar condition for tracking the time duration.

    function rewardPerToken() public view returns (uint256) {
        if (_totalSupply == 0) {
            return rewardPerTokenStored;
        }
        return
            rewardPerTokenStored + (((lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18) / _totalSupply);
    }

Due to this oversight, even when there are no users staking, the accounting logic incorrectly assumes that funds are being dispersed during that duration because the starting timestamp is updated. As a result, if the notifyRewardAmount() function is called before any users are staking, the rewards that should have been allocated to the first stakers instead accrue to no one and become permanently locked in the contract.

Proof of Concept

This test shows the scenario mentioned above:

    function testZeroTotalSupply() public {

        address bob = vm.addr(20);
        stakingToken.mint(bob, 10000 ether);

        vm.startPrank(rewardDistributor);
        rewardToken.transfer(address(staking), 7*86400 ether);
        staking.notifyRewardAmount(7*86400 ether);
        vm.stopPrank();

        console.log("Initial Reward Balance is: ", rewardToken.balanceOf(address(staking)));
        console.log("Initial Staking Balance of Bob is: ", stakingToken.balanceOf(bob));

        vm.warp(block.timestamp + 24 hours);

        vm.startPrank(bob);
        stakingToken.approve(address(staking), 10000 ether);
        staking.stake(10000 ether);
        vm.stopPrank();

        vm.warp(block.timestamp + 6 days);

        vm.startPrank(bob);
        staking.exit();
        assertGt(rewardToken.balanceOf(address(staking)), 0);

        console.log("Final Reward Balance is:   ", rewardToken.balanceOf(address(staking)));
        console.log("Final Staking Balance of Bob is:   ", stakingToken.balanceOf(bob));
    }

The test result is:

Ran 1 test for test/MakerTest.t.sol:MakerTest
[PASS] testZeroTotalSupply() (gas: 327130)
Logs:
  Initial Reward Balance is:  604800000000000000000000
  Initial Staking Balance of Bob is:  10000000000000000000000
  Final Reward Balance is:     86400000000000000000000
  Final Staking Balance of Bob is:    10000000000000000000000

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.51ms (471.30µs CPU time)

Consequently, 86400 ether is locked in the contract, as these rewards were never distributed.

Impact

Non-distributed rewards are stuck in the contract.

Code Snippet

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

Tool used

Manual Review

Recommendation

In the function notifyRewardAmount(), check if there are stakers in the contract:

    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;
        }
+     require(_totalSupply != 0, "No Stakers!");

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

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

Matin - Rewards are distributed even when there are no stakers, resulting in the rewards being permanently locked away #51