Rewards in the UniStaker.sol contract for the initial period can be lost

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-02-uniswap-foundation/blob/main/src/UniStaker.sol#L570-L599

Vulnerability details

Impact

The UniStaker.sol contract within the UniStaker Infrastructure Protocol is designed to manage the staking of UNI tokens and the distribution of rewards generated from protocol fees on Uniswap V3. This contract allows UNI holders to stake their tokens, delegate their governance voting rights, and earn rewards based on the fees collected from Uniswap V3 pools. The rewards are distributed in a designated token, which is determined at the deployment of the contracts.

The staking mechanism is inspired by the Synthetix StakingRewards.sol implementation, which distributes rewards over a fixed period. This period restarts whenever new rewards are added to the system. Stakers can deposit their UNI tokens into the contract, delegate their voting rights, and specify a beneficiary for their rewards. The contract tracks each staker's deposit on a per-position basis, allowing for independent management of each staking position.

Rewards management and distribution are handled through a system where the accrued fees from Uniswap V3 pools are auctioned for the designated reward token. The UniStaker.sol contract collects these tokens and distributes them to stakers based on their share of the total staked UNI. The distribution rate is determined by the amount of reward tokens available and the duration over which they are to be distributed.

The reward calculation and distribution mechanism is designed to ensure fair and proportional allocation of rewards to all stakers based on their contribution to the pool. However, this system relies on the accurate tracking of staking durations and the timely update of reward rates.

The primary impact of the bug in the UniStaker.sol contract is the potential loss of rewards for the initial period of staking. This can occur when the reward distribution mechanism does not account for the time elapsed between the initiation of the reward period and the participation of the first staker.
The vulnerability in the UniStaker.sol contract mirrors the issue found in Synthetix's staking rewards mechanism, where rewards for the initial period can be lost if there are no participants to claim them. In the context of the UniStaker Infrastructure Protocol, this means that rewards generated from protocol fees could go unclaimed during periods of low or no participation in staking. This not only leads to inefficiencies in reward distribution but also disincentivizes early participation in the staking process.

Proof of Concept

• UniStaker.sol#notifyRewardAmount() function

  /// @notice Called by an authorized rewards notifier to alert the staking contract that a new
  /// reward has been transferred to it. It is assumed that the reward has already been
  /// transferred to this staking contract before the rewards notifier calls this method.
  /// @param _amount Quantity of reward tokens the staking contract is being notified of.
  /// @dev It is critical that only well behaved contracts are approved by the admin to call this
  /// method, for two reasons.
  ///
  /// 1. A misbehaving contract could grief stakers by frequently notifying this contract of tiny
  ///    rewards, thereby continuously stretching out the time duration over which real rewards are
  ///    distributed. It is required that reward notifiers supply reasonable rewards at reasonable
  ///    intervals.
  //  2. A misbehaving contract could falsely notify this contract of rewards that were not actually
  ///    distributed, creating a shortfall for those claiming their rewards after others. It is
  ///    required that a notifier contract always transfers the `_amount` to this contract before
  ///    calling this method.
  function notifyRewardAmount(uint256 _amount) external {
    if (!isRewardNotifier[msg.sender]) revert UniStaker__Unauthorized("not notifier", msg.sender);

    // We checkpoint the accumulator without updating the timestamp at which it was updated, because
    // that second operation will be done after updating the reward rate.
    rewardPerTokenAccumulatedCheckpoint = rewardPerTokenAccumulated();

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

    rewardEndTime = block.timestamp + REWARD_DURATION;
    lastCheckpointTime = block.timestamp;

    if ((scaledRewardRate / SCALE_FACTOR) == 0) revert UniStaker__InvalidRewardRate();

    // This check cannot _guarantee_ sufficient rewards have been transferred to the contract,
    // because it cannot isolate the unclaimed rewards owed to stakers left in the balance. While
    // this check is useful for preventing degenerate cases, it is not sufficient. Therefore, it is
    // critical that only safe reward notifier contracts are approved to call this method by the
    // admin.
    if (
      (scaledRewardRate * REWARD_DURATION) > (REWARD_TOKEN.balanceOf(address(this)) * SCALE_FACTOR)
    ) revert UniStaker__InsufficientRewardBalance();

    emit RewardNotified(_amount, msg.sender);
  }

In essence, the issue arises when the contract begins to accrue rewards from the start of a reward period, regardless of whether there are stakers present to claim these rewards. If the reward period starts and continues without any stakers, the accrued rewards during this time are effectively lost from the perspective of potential participants.

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

• 0xMacro Blog - Synthetix Vulnerability
• Same vulnerability from past c4 contest

Tools Used

Manual Review

Recommended Mitigation Steps

A 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 UniStaker.sol#notifyRewardAmount() function.

Assessed type

Context

[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