Summary
The BaseGauge::notifyRewardAmount() function can be called multiple times through GaugeController.distributeRewards(), which overrides the existing reward rate. This resets the reward distribution, causing loss of unclaimed rewards that were being distributed at the previous rate.
Vulnerability Details
The notifyRewardAmount() function simply overwrites the rewardRate state variable without accounting for existing undistributed rewards:
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;
emit RewardNotified(amount);
}
function notifyReward(
PeriodState storage state,
uint256 amount,
uint256 maxEmission,
uint256 periodDuration
) internal view returns (uint256) {
if (amount > maxEmission) revert RewardCapExceeded();
if (amount + state.distributed > state.emission) {
revert RewardCapExceeded();
}
@> uint256 rewardRate = amount / periodDuration;
if (rewardRate == 0) revert ZeroRewardRate();
@> return rewardRate;
}
When called multiple times:
The existing rewardRate is overwritten with the new rate
lastUpdateTime is reset to current timestamp
Previous unclaimed rewards that were being distributed at the old rate are effectively lost
This means that if rewards were being distributed at rate X and a new call sets rate Y, users who haven't claimed their rewards from rate X period will lose them.
Impact
Users lose unclaimed rewards when the reward rate is overridden by subsequent calls to notifyRewardAmount(). This directly leads to loss of funds for stakers who haven't claimed their rewards yet.
Tools Used
Manual review
Proof of Concept
First a bug in the BaseGauge::constructor() function need to be fixed, as this cause overflow in the calculations due to a incorrect assignment of boostState.minBoost:
- boostState.minBoost = 1e18;
+ boostState.minBoost = 10000;
Add the following test case to the test/unit/core/governance/gauges/GaugeController.test.js file:
it("loss of rewards on multiple notify calls", async () => {
await veRAACToken.mint(user1.address, ethers.parseEther("2000"));
await veRAACToken.connect(user1).approve(rwaGauge.target, ethers.parseEther("1000"));
await rwaGauge.connect(user1).stake(ethers.parseEther("1000"));
await gaugeController.connect(user1).vote(rwaGauge.target, 5000);
const initialPeriodState = await rwaGauge.periodState();
expect(initialPeriodState.distributed).to.be.eq(0);
const rewardsToDistribute = ethers.parseEther("500000");
await rewardToken.mint(rwaGauge.target, rewardsToDistribute);
await gaugeController.distributeRewards(rwaGauge.target);
const firstPeriodState = await rwaGauge.periodState();
const generatedRewards = firstPeriodState.distributed;
expect(generatedRewards).to.be.equal(rewardsToDistribute);
const rewardRate = await rwaGauge.rewardRate();
await time.increase(WEEK);
const earned = await rwaGauge.earned(user1.address);
await gaugeController.connect(user1).vote(rwaGauge.target, 10);
await gaugeController.connect(user1).vote(raacGauge.target, 5000);
await gaugeController.distributeRewards(rwaGauge.target);
const newRewardRate = await rwaGauge.rewardRate();
const newEarned = await rwaGauge.earned(user1.address);
expect(newEarned).to.be.lt(earned);
expect(newRewardRate).to.be.lt(rewardRate);
});
Recommendations
Track and account for existing undistributed rewards when updating the reward rate:
function notifyRewardAmount(uint256 amount) external override onlyController updateReward(address(0)) {
if (amount > periodState.emission) revert RewardCapExceeded();
+ uint256 remainingRewards = 0;
+ if (block.timestamp < periodFinish()) {
+ remainingRewards = periodFinish() - block.timestamp * rewardRate;
+ }
+ // Add remaining rewards to new amount
+ uint256 totalRewards = amount + remainingRewards;
- rewardRate = notifyReward(periodState, amount, periodState.emission, getPeriodDuration());
+ rewardRate = notifyReward(periodState, totalRewards, periodState.emission, getPeriodDuration());
periodState.distributed += amount;
uint256 balance = rewardToken.balanceOf(address(this));
if (rewardRate * getPeriodDuration() > balance) {
revert InsufficientRewardBalance();
}
lastUpdateTime = block.timestamp;
emit RewardNotified(amount);
}
