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

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

touthang

Summary

Furure stakers are always vested rewards using the total rewardPerToken without deducting how much rewardPerToken has already been accumulated before they stake.

Any new staker will be rewarded with rewardData.rewardPerTokenStored that have been accumulated since creation of contract. That means a user who hasn't been staking from the beginning will be rewarded the same as a user who has been staking sing the beginning. Or in other words the contract assumes that every user staked when the rewardData.rewardPerTokenStored is 0.

This is exactly the same problem mentioned here by rareskills.

Vulnerability Details

stake calls updateReward modifier and we can see that every new staker's userRewardPerTokenPaid will be 0 since vesting rate is currently 0;

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;
}
}
_;
}
function _getVestingRate(StakeInfo memory _stakeInfo) internal view returns (uint256 vestingRate) {
if (_stakeInfo.stakeTime == 0) {
return 0;
}
if (block.timestamp > _stakeInfo.fullyVestedAt) {
vestingRate = 1e18;
} else {
vestingRate = (block.timestamp - _stakeInfo.stakeTime) * 1e18 / vestingPeriod;
}
}

Suppose,

  • after 1 year the accumulated rewardPerTokenStored is 1000e18, which means every token staked since the beginning has earned 1000 tokens each.

  • a new user Bob stakes 100 TEMPLE tokens.

  • assuming the vesting period is 16 weeks, Bob claim his rewards after 16 weeks

  • now his reward calculation is;

_perTokenReward = _rewardPerToken();

we can see that __perTokenReward for the new staker is the total __rewarsPerToken which has been accumulating since the beginning.

return
(_stakeInfo.amount * (_perTokenReward - userRewardPerTokenPaid[_account][_index])) / 1e18 +
claimableRewards[_account][_index];
}

  • (100 TEMPLE * (1000e18 - 0)) / 1e18 + 0

  • = 100,000 TGLD rewards for staking 16 weeks.

This is assuming no new rewards are added, where bob should've gotten 0 TGLD as rewards. Bob just earned rewards equivalent to staking 1 year + 16 weeks(for new rewards).

The issue here is that the contract does not account for the already accumulated rewardPerTokenStored from the past when calculating the new staker's userRewardPerTokenPaid and calculates it using the total rewardPerToken :

userRewardPerTokenPaid[_account][_index] = vestingRate * uint256(rewardData.rewardPerTokenStored) / 1e18;.

Impact

Loss of reward tokens and unfair reward calculation for initial stakers or DOS(not enough reward tokens).
It also opens an attack path where a user can steal unclaimed rewards even when there are no new rewards added i.e. a user who enters after no new rewards are distributed will still get rewards from the past.

Tools Used

manual

Recommendations

Introduce a new variable;

//@audit a new variable to keep track of the already accumulated rewardPerToken before the user enters the protocol
mapping(address account => mapping(uint256 index => uint256 amount)) public userRewardDebt;

note that we cannot use the variable userRewardPerTokenPaid to track the already accumulated rewardPerToken because of the vestingRate, that would introduce another problem.

function stakeFor(address _for, uint256 _amount) public whenNotPaused {
if (_amount == 0) revert CommonEventsAndErrors.ExpectedNonZero();
​
stakingToken.safeTransferFrom(msg.sender, address(this), _amount);
uint256 _lastIndex = _accountLastStakeIndex[_for];
_accountLastStakeIndex[_for] = ++_lastIndex;
​
+ userRewardDebt[_for][_lastIndex] = _rewardPerToken(); //@audit we need to make sure we do it here before _applyStake, because it calls updateReward
​
_applyStake(_for, _amount, _lastIndex);
_moveDelegates(address(0), delegates[_for], _amount);
}
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;
+ //@audit vest only the newly accrued rewardPerToken after the user enters
+ userRewardPerTokenPaid[_account][_index] = vestingRate * (uint256(rewardData.rewardPerTokenStored) - userRewardDebt[_account][_index] ) / 1e18;
// this will still be zero when new staker enters, but since we deduct the userRewardDebt before calculating it, we will only reward with the newly accrued rewardPerToken
}
}
_;
}
function _earned(
StakeInfo memory _stakeInfo,
address _account,
uint256 _index
) internal view returns (uint256) {
uint256 vestingRate = _getVestingRate(_stakeInfo);
if (vestingRate == 0) {
return 0;
}
uint256 _perTokenReward;
if (vestingRate == 1e18) {
- _perTokenReward = _rewardPerToken();
+ _perTokenReward = _rewardPerToken() - userRewardDebt[_account][_index];
} else {
- _perTokenReward = _rewardPerToken() * vestingRate / 1e18;
+ _perTokenReward = (_rewardPerToken() - userRewardDebt[_account][_index]) * vestingRate / 1e18;
}
return
(_stakeInfo.amount * (_perTokenReward - userRewardPerTokenPaid[_account][_index])) / 1e18 +
claimableRewards[_account][_index];
}

This ensures that _perTokenReward for the staker is only the newly accumulated rewardPerToken, post staking.

Updates

Lead Judging Commences

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