Core Contracts

Summary

After the intended periodFinish, the rewardPerTokenStored must not be updated. But, in this case, periodFinish keeps getting pushed. So, the rewardPerTokenStored is incremented even after the intended periodFinish . This would inflate the earned value of the user.

Vulnerability Details

lastTimeRewardApplicable is defined as follows:

This implies that if block.timestamp > periodFinish , we should always return periodFinish from lastTimeRewardApplicable function.

So, in this calculation in getRewardPerToken:

If lastUpdateTime was updated to be equal to periodFinish previously, lastTimeRewardApplicable should ideally return periodFinish, and the subtraction (lastTimeRewardApplicable() - lastUpdateTime) must be 0.

So, rewardPerTokenStored should not be updated, when the periodFinish is reached and lastUpdateTime was set equal to periodFinish previously.

But in this case, when a periodFinish is fixed, lastTimeRewardApplicable will actually return a value greater than the intended period finish. This is because whenever stake, withdraw or getReward is called lastUpdateTime is set to block.timestamp, which increases the periodFinish value.

_updateRewardis called whenever stake, withdrawor getRewardis called:

And periodFinishis defined like this:

As lastUpdateTimeis incremented with every stake, withdrawor getReward call, periodFinish keeps getting extended. So, if the intended period finish was 7 days, the periodFinish will become more than 7 days, if users call stakefunction.

So, lastTimeRewardApplicable can return a value greater than the intended periodFinish. This implies that rewardPerTokenStored will get incremented even after the intended period finishes, when it should not be incremented:

Impact

If rewardPerTokenStored is incremented, it will result in an inflated value of earned for the user, allowing users to claim more than intended as earnedis directly dependent on rewardPerTokenStored or getRewardPerToken:

Tools Used

Manual review

Recommendations

periodFinish should not be incremented every time lastUpdateTimeis set. This would indefinitely increase the periodFinish. periodFinishshould only be set when notifyRewardsis called.