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

Users will earn the whole `_rewardPerToken` even if they stake near the end

pyro

Summary

_applyStake doesn't sync userRewardPerTokenPaid with _rewardPerToken, allowing any staker to claim the full _rewardPerToken amount for their balance.

Vulnerability Details

When staking with stakeFor, users are assigned to their old, but empty index (no stakes on this index).
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGoldStaking.sol#L265

uint256 _lastIndex = _accountLastStakeIndex[_for];
_accountLastStakeIndex[_for] = ++_lastIndex;
_applyStake(_for, _amount, _lastIndex);

After that, _applyStake is called to update the _stakeInfos map. Notice how the updateReward modifier is called on _applyStake, but because it's a modifier, it's called before we update the _stakeInfos map.
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGoldStaking.sol#L495

function _applyStake(address _for, uint256 _amount, uint256 _index) internal updateReward(_for, _index) {
totalSupply += _amount;
_balances[_for] += _amount;
_stakeInfos[_for][_index] = StakeInfo(uint64(block.timestamp), uint64(block.timestamp + vestingPeriod), _amount);
emit Staked(_for, _amount);
}

Since it's called before _stakeInfos is initialized, it will calculate claimableRewards and userRewardPerTokenPaid as 0.
https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGoldStaking.sol#L594

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;
}

However, after the _applyStake call, our user has _stakeInfos map with their balances, but userRewardPerTokenPaid is 0. Now they would be able to call getReward and claim the full _rewardPerToken, even though they just staked.

Impact

Loss of funds. Users game the system.

Tools Used

Manual review

Recommendations

Inside _applyStake, manually sync userRewardPerTokenPaid to _rewardPerToken. This may require more code, as we also need to account for vesting.

Updates

Lead Judging Commences

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

Appeal created

pyro Submitter
11 months ago
inallhonesty Lead Judge
11 months ago
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.