Rough Brick Meerkat
Medium
In the function notifyRewardAmount, calculation of scaledRewardRate is in error when the rewards are notified prior to the scheduled rewardEndTime.
Refer to the function notifyRewardAmount in the GovernanceStaker contract. When the authorized rewards notifier notifies the staking contract that a newreward has been transferred to it prior to the scheduled rewardEndTime, the function computes _remainingReward using the stale scaledRewardRate for the remaining period. So this computed _remainingReward is in error. This _remainingReward is added to the scaled reward _amount for the computation of the new scaledRewardRate, which also becomes erroneous.
The correct approach is to calculate the new scaledRewardRate is to use the available data itself. Please refer to the timeline below
|--------------------------------------------|----------|
where the three vertical lines above are labelled 1, 2 and 3 starting from the left hand side and correspond to 1 = Previous reward notification time 2 = timestamp of notification (block.timestamp) 3 = End time (rewardEndTime)
time difference between 1 to 3 gives REWARD_DURATION
The notification is received at timeline 2. Thus the notified 'amount' corresponds to the interval between timeline points 1 and 2. This interval only should be used in the calculation of the scaledRewardRate as is done in the 'if' case in the code at lines #439 to #440. Calculation of this interval follows
Previous reward notification time + REWARD_DURATION = End time;
Previous reward notification time = End time - REWARD_DURATION;
time elapsed since Previous reward notification time = timestamp of notification - Previous reward notification time
time elapsed since Previous reward notification time = timestamp of notification - (End time - REWARD_DURATION)
time elapsed since Previous reward notification time = timestamp of notification + REWARD_DURATION - End time;
It is this value of time elapsed since Previous reward notification time which should be used for the computation of scaledRewardRate. The quantity of reward tokens the staking contract is being notified of (_amount) corresponds to this period of time. Thus,
scaledRewardRate = (_amount * SCALE_FACTOR) / (timestamp of notification + REWARD_DURATION - End time);
if (block.timestamp >= rewardEndTime) {
scaledRewardRate = (_amount * SCALE_FACTOR) / REWARD_DURATION;
} else {
uint256 _remainingReward = scaledRewardRate * (rewardEndTime - block.timestamp);
scaledRewardRate = (_remainingReward + _amount * SCALE_FACTOR) / REWARD_DURATION;
}
Depends on the time gap from the previous notification of rewards and the numbers of stake token either being added or withdrawn since previous notification (referred as Previous reward notification time in the Vulnerability Detail section). So the impact can vary from nil to very high.
Manual Review
As explained in the finding, the modified code can be :
if (block.timestamp >= rewardEndTime) {
scaledRewardRate = (_amount * SCALE_FACTOR) / REWARD_DURATION;
} else {
scaledRewardRate = (_amount * SCALE_FACTOR) / (block.timestamp + REWARD_DURATION – rewardEndTime);
}