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

Incorrect period finish calculation leads to unintended reward distribution after period end

3n0ch

Relevant Context

The BaseGauge contract implements staking and reward distribution functionality. Users can stake tokens and earn rewards over time. The reward distribution period is meant to be fixed (7 days by default) when rewards are notified via notifyRewardAmount. The reward rate is calculated by dividing the reward amount by the period duration.

Finding Description

The BaseGauge#periodFinish function incorrectly calculates the end time of the reward period by adding the period duration to lastUpdateTime. Since lastUpdateTime is updated in the updateReward modifier (which is used in many functions like stake, withdraw, etc.), this causes the period end time to extend each time these functions are called.

The root cause is in BaseGauge#periodFinish:

function periodFinish() public view returns (uint256) {
return lastUpdateTime + getPeriodDuration();
}

This affects reward calculations in getRewardPerToken which uses lastTimeRewardApplicable() that depends on periodFinish():

function getRewardPerToken() public view returns (uint256) {
if (totalSupply() == 0) {
return rewardPerTokenStored;
}
return rewardPerTokenStored + (
(lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18 / totalSupply()
);
}

Impact Explanation

Medium. The reward rate remains fixed as set in notifyRewardAmount, but the incorrect period finish calculation allows rewards to continue being distributed after the intended period end, as long as there are remaining rewards in the contract. This affects the intended reward distribution schedule but does not lead to loss of funds or manipulation of reward rates.

Likelihood Explanation

High. This occurs whenever there are remaining rewards in the contract after the intended period end, and users continue to interact with the contract through functions with the updateReward modifier.

Proof of Concept

  1. At t=100, admin calls notifyRewardAmount with 700 tokens for a 7-day period

    • Reward rate is set to 100 tokens/day

    • Intended period finish: t=107

  2. At t=107 (period end), 100 tokens remain unclaimed in the contract

    • Due to various factors, such as users not claiming,...

  3. At t=108, User A calls stake

    • updateReward modifier updates lastUpdateTime to 108

    • periodFinish now returns 115 (108 + 7)

    • The remaining rewards continue to be distributed using the same reward rate, despite the intended period being over

The key point is that the reward rate remains constant as set in notifyRewardAmount, but the distribution period incorrectly extends, allowing distribution of any remaining rewards beyond the intended period end.

Recommendation

Store the period finish timestamp explicitly and handle the case where lastTimeRewardApplicable() is less than lastUpdateTime:

contract BaseGauge {
+ uint256 public rewardPeriodFinish;
​
function notifyRewardAmount(uint256 amount) external override onlyController updateReward(address(0)) {
if (amount > periodState.emission) revert RewardCapExceeded();
rewardRate = notifyReward(periodState, amount, periodState.emission, getPeriodDuration());
periodState.distributed += amount;
uint256 balance = rewardToken.balanceOf(address(this));
if (rewardRate * getPeriodDuration() > balance) {
revert InsufficientRewardBalance();
}
lastUpdateTime = block.timestamp;
+ rewardPeriodFinish = block.timestamp + getPeriodDuration();
emit RewardNotified(amount);
}
​
function periodFinish() public view returns (uint256) {
- return lastUpdateTime + getPeriodDuration();
+ return rewardPeriodFinish;
}
​
function getRewardPerToken() public view returns (uint256) {
if (totalSupply() == 0) {
return rewardPerTokenStored;
}
​
+ uint256 lastApplicable = lastTimeRewardApplicable();
+ if (lastApplicable <= lastUpdateTime) {
+ return rewardPerTokenStored;
+ }
​
return rewardPerTokenStored + (
- (lastTimeRewardApplicable() - lastUpdateTime) * rewardRate * 1e18 / totalSupply()
+ (lastApplicable - lastUpdateTime) * rewardRate * 1e18 / totalSupply()
);
}
}

This change prevents potential underflow in the reward calculation and ensures rewards are correctly calculated when the period has finished.

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!