calcaultion of scaledRewardRate will make the rewards stuck in the contract when totalStaked = 0

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/5a2761c8277541a24bc551fbd624413b384bea94/src/UniStaker.sol#L577-L582

Vulnerability details

Impact

scaledRewardRate is only calculated in notifyRewardAmount without considering if totalStaked is 0 or not. And scaledRewardRate is not updated anywhere else. So if totalStaked = 0, this will cause the waste of reward tokens, as these tokens are considered to be already distributed during the period but they are actually not. This leads to the waste and lock of funds.

Proof of Concept

scaledRewardRate is used to represent the global rate at which rewards are currently being distributed to stakers. It is updated every time there is a new reward coming in and notifying via notifyRewardAmount.

    if (block.timestamp >= rewardEndTime) {
      scaledRewardRate = (_amount * SCALE_FACTOR) / REWARD_DURATION;
    } else {
      uint256 _remainingReward = scaledRewardRate * (rewardEndTime - block.timestamp);
      scaledRewardRate = (_remainingReward + _amount * SCALE_FACTOR) / REWARD_DURATION;
    }

The problem is that, if totalStaked = 0, rewards have been distributed according to the calculation _remainingReward = scaledRewardRate * (rewardEndTime - block.timestamp) but they are not rewarded since no one is staking yet. The root cause is that rewards are still distributed even though no one is staking.

Since scaledRewardRate can not be updated elsewhere, these reward tokens are certainly wasted as they won't be counted in the next reward period. This leads to the waste and lock of funds.

Note: Several cases will all cause totalStaked=0, like no one has staked before, or the only staker decides to withdraw all shares.

The PoC is shown below (in UniStaker.t.sol::NotifyRewardAmount):

  function test_FundLoss() public {
    uint256 _amount = 1 ether;
    _mintTransferAndNotifyReward(_amount);
    uint256 _expectedRewardRate = (SCALE_FACTOR * _amount) / uniStaker.REWARD_DURATION();
    assertEq(uniStaker.scaledRewardRate(), _expectedRewardRate);
    emit log_named_uint("totalStake during 1st pharse of reward", uniStaker.totalStaked());
    emit log_named_uint("scaledRewardRate in the first phase",uniStaker.scaledRewardRate());
    vm.warp(block.timestamp + 15 days);
    _mintTransferAndNotifyReward(_amount);
    emit log_named_uint("scaledRewardRate in the second phase",uniStaker.scaledRewardRate());
    emit log_named_uint("scaledRewardRate with 1.5 * _amount in the second phase",_expectedRewardRate * 3 / 2);
    emit log_string("0.5 * _amount reward has been lost");
  }

The log is shown below, even though there is no staker, half of the _amount (15 days / 30 days) is still considered to be consumed:

Logs:
  totalStake during 1st pharse of reward: 0
  scaledRewardRate in the first phase: 385802469135802469135802469135802469135802469135
  scaledRewardRate in the second phase: 578703703703703703703703703703703703703703703703
  scaledRewardRate with 1.5 * _amount in the second phase: 578703703703703703703703703703703703703703703702
  0.5 * _amount reward has been lost

Tools Used

Foundry

Recommended Mitigation Steps

I would suggest the following:

1. If totalStaked is 0, the rewarding process should not be open, and the fund could be recorded in a variable nextRoundRewards for the next reward process.
2. If the only staker quits, the rewarding process should also stop, and the remaining fund should also be recorded in nextRoundRewards.

Assessed type

Math

[c4-judge]

MarioPoneder marked the issue as duplicate of #9

[c4-judge]

MarioPoneder marked the issue as duplicate of #369

[c4-judge]

MarioPoneder marked the issue as unsatisfactory:
Invalid