TempleGold

TempleDAO
Foundry
25,000 USDC
View results
Submission Details
Severity: medium
Valid

Modifying the vesting period can lead to "over boosted" staking

Summary

A wrong assumption in how vesting rates are calculated can lead to boosts over the 100% rate when vesting periods are adjusted.

Vulnerability Details

TempleGold staking works with a vesting mechanism. Depositors get increasing rewards as their vesting gets realized over the period defined by vestingPeriod. The effective rate is given by the _getVestingRate() function.

484: function _getVestingRate(StakeInfo memory _stakeInfo) internal view returns (uint256 vestingRate) {
485: if (_stakeInfo.stakeTime == 0) {
486: return 0;
487: }
488: if (block.timestamp > _stakeInfo.fullyVestedAt) {
489: vestingRate = 1e18;
490: } else {
491: vestingRate = (block.timestamp - _stakeInfo.stakeTime) * 1e18 / vestingPeriod;
492: }
493: }

If the current timestamp is past the fully vested timestamp, the rate is 100% (1e18). Else, a linear progression is applied over the vesting period. Note here that, while fullyVestedAt is determined during stake time in _applyStake(), the vestingPeriod variable is read on the fly with the value it currently has at that point in time.

The vesting period time can be changed by protocol owners using setVestingPeriod(). Changing the vesting period while having unrealized vestings (i.e. vestings that have not reached the fullyVestedAt timestamp) can lead to undefined behavior.

Let's say the vesting period is currently at 3 weeks. Someone deposits at t and will have its stake fully vested at t + 3 weeks. However, at time t + 2 weeks the protocol changes the vesting period to 1 week. The stake information will still have its fully vested time at t + 3 weeks. This means that between t + 2 weeks and t + 3 weeks the vesting rate for this stake will be calculated according to line 491, i.e. (t + 2 weeks - t) * 1e18 / 1 week = 2e18, a boost of 2x, the double of the expected maximum.

Note that the vesting period must be changed after the current distribution has ended, but this is independent of the staking. A user can have a non-fully-realized stake at any point in time.

Proof of Concept

Refer to the example detailed above as a proof of concept of the issue.

Impact

Changing the vesting period can lead to over boosted rates and users receiving more rewards than intended. This over-distribution in rewards will cause multiple cascading accounting issues that will eventually lead to users not being able to claim their honest and intended rewards.

Tools Used

None.

Recommendations

Store the original vesting period in the StakeInfo struct, and use this value in _getVestingRate(). Another alternative would be to always cap the rate to 1e18.

Updates

Lead Judging Commences

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

Changes to vesting period is not handled inside `_getVestingRate`

Support

FAQs

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