Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

`periodFinish` can be incremented indefinitely leading to an inflated value of `earned` rewards for the user.

SlayerSecurity

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:

function lastTimeRewardApplicable() public view returns (uint256) {
return block.timestamp < periodFinish() ? block.timestamp : periodFinish();
}

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

So, in this calculation in getRewardPerToken:

return rewardPerTokenStored + (
(lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18 / totalSupply()
);

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:

function _updateReward(address account) internal {
rewardPerTokenStored = getRewardPerToken();
lastUpdateTime = lastTimeRewardApplicable(); // lastUpdateTime is updated here
if (account != address(0)) {
UserState storage state = userStates[account];
state.rewards = earned(account);
state.rewardPerTokenPaid = rewardPerTokenStoreds;
state.lastUpdateTime = block.timestamp;
emit RewardUpdated(account, state.rewards);
}
}

And periodFinishis defined like this:

function periodFinish() public view returns (uint256) {
return lastUpdateTime + getPeriodDuration(); //periodFinish keeps getting extended
}

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:

return rewardPerTokenStored + (
(lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18 / totalSupply()
); // rewardPerTokenStored incremented even after period finish as lastTimeRewardApplicable
// returns a value more than the intended periodFinish

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:

function earned(address account) public view returns (uint256) {
return (getUserWeight(account) *
(getRewardPerToken() - userStates[account].rewardPerTokenPaid) / 1e18
) + userStates[account].rewards;
}

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.

Updates

Lead Judging Commences

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BaseGauge period end time miscalculation creates circular dependency between periodFinish() and lastUpdateTime, preventing periods from naturally ending and disrupting reward distribution

inallhonesty Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BaseGauge period end time miscalculation creates circular dependency between periodFinish() and lastUpdateTime, preventing periods from naturally ending and disrupting reward distribution

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!