TempleGold

Vulnerability Details:

The stake and stakeFor functions in the TempleGoldStaking contract allow users to stake Temple and claim rewards in Temple Gold. The _applyStake function saves the user's stakeTime, fullyVestedAt, and amount in _stakeInfos. The fullyVestedAt time is particularly important as it ensures that if the vesting period is changed later on, it will not affect already staked users.

The problem is that the _getVestingRate function still uses the vestingPeriod state variable when calculating a user's vesting rate. Therefore, if the vestingPeriod is changed, it will still affect the vesting rates of users who staked before the change.

Consider the following scenarios:

1. Scenario 1:

   • Initial Settings: Vesting rate and reward duration are set to 10 days. Alice stakes after 5 days.

   • Vesting Period Change: After 5 more days, the reward duration ends, allowing changes to the vesting rate and reward duration. The vesting rate and reward duration are updated to 20 days.

   • Alice Claims Rewards: When Alice claims her rewards, the _getVestingRate function is called, and the calculation is performed as follows:

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

     • With the new vesting period of 20 days, the calculation is:

     • 5 days (432000 seconds) * 1e18 / 20 days (1.728e+6 seconds) = 0.25e18 (25%)

   • Impact: Alice should be at a 50% vesting rate because she has staked for 5 out of 10 days, but due to the change, she is incorrectly at 25%, leading to her claiming fewer rewards than she should have.

2. Scenario 2:

   • Initial Settings: Vesting rate and reward duration are set to 20 days. Bob stakes after 5 days.

   • Vesting Period Change: After 15 days, the reward duration ends, allowing changes to the vesting rate and reward duration. The vesting rate and reward duration are updated to 10 days.

   • Bob Claims Rewards: When Bob claims his rewards, the _getVestingRate function is called, and the calculation is performed as follows:

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

     • With the new vesting period of 10 days, the calculation is:

     • 15 days (1.296e+6 seconds) * 1e18 / 10 days (864000 seconds) = 1.5e18 (150%)

   • Impact: Bob should be at a 75% vesting rate because he has staked for 15 out of 20 days, but due to the change, he is incorrectly at 150%, leading to an overestimation of rewards.

   • Additional Impact: Since Bob's vesting rate exceeds the maximum (1e18), once his vesting period is over, his next claim could underflow as userRewardPerTokenPaid could be larger than _perTokenReward.

Impact:

Users who staked under a previous vesting period may receive more or fewer rewards than intended. Overestimations in vesting rates could also lead to underflow issues in reward calculations, causing further problems.

Tools Used:

Manual analysis

Recommendation:

To resolve this issue, ensure that the vestingPeriod used in _getVestingRate is the one that was in effect when the user initially staked. This can be achieved by storing the vestingPeriod in the StakeInfo struct and using it in the calculation.