code-423n4 / 2024-02-uniswap-foundation-findings

2 stars 3 forks source link

Denial of Service when Reward Value is below a Certain Range #94

Closed c4-bot-4 closed 6 months ago

c4-bot-4 commented 6 months ago

Lines of code

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

Vulnerability details

Impact

Denial of Service when notifyRewardAmount(...) function is called with Reward Value that is below 2.592 * 10**6

Proof of Concept

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

...

} as noted from the pointer in the code above, a validation is done to ensure scaledRewardRate / SCALE_FACTOR is not equal zero, to understand this revert condition better: scaledRewardRate is equal _amount SCALE_FACTOR) / REWARD_DURATION, therefore (scaledRewardRate / SCALE_FACTOR) will equal _amount / REWARD_DURATION. Since REWARD_DURATION is 30 days, the implication of this revert condition and calculations explained above is that reward _amount must be greater than 30 24 60 60 which is a representation of 30days in seconds i.e 2.592 10^6 . Therefore whenever input reward amount is less than 2.592 10**6 , it would revert and cause denial of service.

Tools Used

Manual Review

Recommended Mitigation Steps

It should be noted that calling the notifyRewardAmount(...) function with too little amount has it complications however setting the minimum limit to 2.592 * 10**6 is too large, a smaller value should be used by the protocol to avoid denial of service

Assessed type

DoS

MarioPoneder commented 6 months ago

Misbehaving reward notifier, OOS see README:

Publicly Known Issues

  1. A misbehaving reward notifier 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 reward notifier 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.
c4-judge commented 6 months ago

MarioPoneder marked the issue as unsatisfactory: Out of scope