[H-01] Calling notifyRewardAmount will lead to lose of yield for users

[ddimitrov22]

Severity

Impact: High, because users` yield can be manipulated

Likelihood: Medium, because a malicious notifier can call it as many times as he wants

Description

The notifyRewardAmount function takes a yield amount and extends the periodFinish to now + yieldDuration:

periodFinish = block.timestamp + yieldDuration;

If the block.timestamp is less than periodFinish, it will enter the following else block:

        } else {
            uint256 remaining = periodFinish - block.timestamp;
            uint256 leftover = remaining * yieldRate;
            yieldRate = (_amount + leftover) / yieldDuration;
        }

It rebases the yield amount and the leftover yields over the yieldDuration period.

This can lead to diluting the yield rate and rewards being dragged out forever by malicious new yield deposits.

Let's take a look at the following example:

1. For the sake of the example, imagine the current yieldRate is 1000 yield / yieldDuration.
2. When 10% of yieldDuration has passed, a malicious notifier calls notifyRewards with _amount = 0.
3. The new yieldRate = 0 + 900 / yieldDuration, which means the yieldRate just dropped by 10%.
4. This can be repeated infinitely. After another 10% of yield time passed, they trigger notifyRewardAmount(0) to reduce it by another 10% again:
   yieldRate = 0 + 720 / yieldDuration.

The yieldRate should never decrease by a notifyRewardAmount call.

Recommendations

There are two potential fixes to this issue:

1. If the periodFinish is not changed at all and not extended on every notifyRewardAmount call. The yieldRate should just increase by yieldRate += amount / (periodFinish - block.timestamp).

2. Keep the yieldRate constant but extend periodFinish time by += amount / yieldRate.

[wankhede04]

Acknowledge