TempleGold
TempleDAO
Foundry
25,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

setRewardDuration and setVestingPeriod can be griefed from anyone when distributionStarter is unset

SBSecurity

Summary

When distributeRewards can be called by anyone, no new reward duration from setRewardDuration or new vesting period from setVestingPeriod can be called, because it will always be griefed, and periodFinish will be reset, reverting the transactions.

## Vulnerability Details

The issue will happen when distributeRewards is permissionless and everyone can call it, additionally setting a new vesting period and reward duration requires rewardData::periodFinish to be strictly less than the current block.timestamp :

function setVestingPeriod(uint32 _period) external override onlyElevatedAccess {
if (_period < WEEK_LENGTH) { revert CommonEventsAndErrors.InvalidParam(); }
// only change after reward epoch ends
if (rewardData.periodFinish >= block.timestamp) { revert InvalidOperation(); }
vestingPeriod = _period;
emit VestingPeriodSet(_period);
}
​
/**
* @notice Set reward duration
* @param _duration Reward duration
*/
function setRewardDuration(uint256 _duration) external override onlyElevatedAccess {
// minimum reward duration
if (_duration < WEEK_LENGTH) { revert CommonEventsAndErrors.InvalidParam(); }
// only change after reward epoch ends
if (rewardData.periodFinish >= block.timestamp) { revert InvalidOperation(); }
rewardDuration = _duration;
emit RewardDurationSet(_duration);
}

But in the distribute function we are always setting the periodFinish to the current timestamp, which is the root cause of the issue:

internally called by distributeRewards
function _notifyReward(uint256 amount) private {
...MORE CODE
rewardData.lastUpdateTime = uint40(block.timestamp);
rewardData.periodFinish = uint40(block.timestamp + rewardDuration);
}

Here is a POC demonstrating that both functions will revert with InvalidOperation:

forge test --match-test test_griefSetter

function test_griefSetter() public {
vm.startPrank(executor);
staking.setMigrator(alice);
uint256 _rewardDuration = 16 weeks;
uint32 _vestingPeriod = uint32(_rewardDuration);
_setVestingFactor(templeGold);
_setVestingPeriod(_vestingPeriod);
_setRewardDuration(_rewardDuration);
​
// bob stakes
vm.startPrank(bob);
deal(address(templeToken), bob, 1000 ether, true);
_approve(address(templeToken), address(staking), type(uint).max);
staking.stake(100 ether);
vm.stopPrank();
​
skip(2 days);
staking.distributeRewards();
​
vm.startPrank(executor);
uint256 duration = 16 weeks;
staking.setRewardDuration(duration);
}

Impact

Permanently blocking authorized entities from setting new duration and vesting periods.

Tools Used

Manual review

Recommendations

The simplest approach allows setting these values when periodFinish is equal to the current timestamp.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

setRewardDuration and setVestingPeriod can be griefed from anyone when distributionStarter is unset

Appeal created

blckhv Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

setRewardDuration and setVestingPeriod can be griefed from anyone when distributionStarter is unset

Support

FAQs

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