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

Mutable Vesting Period Allows Retroactive Manipulation of Stake Rewards

mrudenko

Summary

The TempleGoldStaking contract allows the admin to change the vestingPeriod at any time. This can lead to unexpected changes in the vesting rates for existing stakes, potentially benefiting some users while disadvantaging others. In extreme cases, it could lead to contract malfunctions.

Vulnerability Details

The vesting rate is calculated using the formula:

vestingRate = (block.timestamp - _stakeInfo.stakeTime) * 1e18 / vestingPeriod

When the vestingPeriod is changed, this directly affects the vesting rate for all existing stakes. This can result in:

  • Accelerated vesting if the period is shortened

  • Delayed vesting if the period is extended

  • Inconsistent vesting rates for stakes made at different times

  • Potential division by zero if vestingPeriod is set to 0
    Examples:
    30-day stake, changed to 20 days after 15 days:

  • Before: 50% vested

  • After: 75% vested
    30-day stake, changed to 40 days after 25 days:

  • Before: 83.33% vested

  • After: 62.5% vested

Impact

  • Users may receive more or fewer rewards than initially expected

  • Inconsistent vesting rates across different stakes

  • Potential for admin to manipulate vesting rates to benefit or harm specific users

  • Risk of contract malfunction if vestingPeriod is set to 0

Tools Used

Manual code review

Recommendations

  • Implement a vesting schedule that is immutable for each stake:

struct StakeInfo {
uint64 stakeTime;
uint64 vestingPeriod;
uint256 amount;
}

  • When changing the vestingPeriod, only apply it to new stakes:

function setVestingPeriod(uint32 _period) external onlyElevatedAccess {
if (_period < WEEK_LENGTH) { revert CommonEventsAndErrors.InvalidParam(); }
newVestingPeriod = _period;
emit NewVestingPeriodSet(_period);
}
​
function stake(uint256 amount) external override {
// ... existing code ...
_stakeInfos[msg.sender][_index] = StakeInfo(
uint64(block.timestamp),
uint64(newVestingPeriod),
_amount
);
// ... rest of the code ...
}

  • Implement a minimum value check for vestingPeriod to prevent it from being set to 0:

require(_period >= MINIMUM_VESTING_PERIOD, "Vesting period too short");

  • Consider implementing a time-delay or governance vote for changing vesting parameters to allow users to react to upcoming changes.

Updates

Lead Judging Commences

inallhonesty Lead Judge 12 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.