TempleGold

TempleDAO
Foundry
25,000 USDC
View results
Submission Details
Severity: high
Invalid

TempleGoldStaking contract treat new staker and old staker the same, hence have the same reward token distributed

Summary

TempleGoldStaking contract treat new staker and old staker the same, hence have the same reward token distributed

Vulnerability Details

In TempleGoldStaking contract, we have a mapping called userRewardPerTokenPaid, which basically caches staker's _perTokenReward since the last time the staker interacted with the contract

When staker stakes at any time, userRewardPerTokenPaid[_account][_index] is not set, which means the next time user claim the reward, the protocol treats the staker as if he staked at the very beginning of the staking start point

POC: throw this to TempleGoldStaking.t.sol:

function test_letsee() public{
// for distribution
skip(3 days);
uint32 _vestingDuration = 2 weeks;
uint32 _rewardDuration = 16 weeks;
_setVestingPeriod(_vestingDuration);
_setRewardDuration(_rewardDuration);
_setVestingFactor(templeGold);
vm.startPrank(alice);
deal(address(templeToken), alice, 1000 ether, true);
_approve(address(templeToken), address(staking), type(uint).max);
uint256 stakeAmount = 100 ether;
staking.stake(stakeAmount);//alice stake 100 ether with stakingIndex = 1
uint256 goldBalanceBefore = templeGold.balanceOf(address(staking));
staking.distributeRewards();
uint256 goldRewardsAmount = templeGold.balanceOf(address(staking)) - goldBalanceBefore;
emit log_uint(goldRewardsAmount);
skip(5 days);
staking.stake(stakeAmount);//alice stake again 100 ether with stakingIndex = 2
skip(3 weeks);// wait for both 2 staking are fully vested
{
uint256 cacheBalance = templeGold.balanceOf(alice);
staking.getReward(alice, 1);
uint256 rewardStake1 = templeGold.balanceOf(alice) - cacheBalance;
emit log_uint(rewardStake1);
cacheBalance = templeGold.balanceOf(alice);
staking.getReward(alice, 2);
uint256 rewardStake2 = templeGold.balanceOf(alice) - cacheBalance;
emit log_uint(rewardStake2);
//2 staking reward is the same!!!
assertEq(rewardStake1,rewardStake2);
}
}

Impact

This will have an unfair scenario:

  • VESTING_DURATION = 2 weeks, REWARD_DURATION = 16 weeks

  • Alice stake() 1000e18 stakeToken in week 1

  • Bob stake() 1000e18 stakeToken in week 14

  • In week 16, Alice and Bob getReward(), and both have the same rewardToken amount

More extremely, Bob in the above scenario can loan a gigantic amount of stakeToken to stake() in week 14, which he will end up taking most of the reward token in that staking period

Tools Used

Manual Review

Recommendations

Make sure the staker's _accountLastStakeIndex is set while staker calls to stake()

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Appeal created

0xhuy0512 Submitter
11 months ago
inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Too generic

Support

FAQs

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