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

Tokens can get stuck because of limiting recover functionality

dianivanov

Summary

The recoverToken function has limitations for rewardToken and stakingToken which are understandable, but they are both prone to accidential transfer.(because TempleGoldStaking is whitelisted for TempleGOLD) . If these tokens are accidentally transferred directly without using the staking workflow (stakeor stakeForfunction) the tokens will be permanently locked in the TempleGoldStaking contract.

Vulnerability Details

The recoverToken function in the contract does not permit the recovery of staking/reward tokens. This means that if these tokens are accidentally transferred directly to the contract, they cannot be retrieved, leading to a permanent lock.

https://github.com/Cyfrin/2024-07-templegold/blob/57a3e597e9199f9e9e0c26aab2123332eb19cc28/protocol/contracts/templegold/TempleGoldStaking.sol#L381

function recoverToken(address _token, address _to, uint256 _amount) external override onlyElevatedAccess {
> if (_token == address(stakingToken) || _token == address(rewardToken ))
> { revert CommonEventsAndErrors.InvalidAddress(); }
​
IERC20(_token).safeTransfer(_to, _amount);
emit CommonEventsAndErrors.TokenRecovered(_to, _token, _amount);
}

The function should be able to recover an amount of staking/reward tokens if accidentally sent, ensuring no more amount than balanceOf(stakingToken) - totalSupply for the stakingToken. Additionally, since TempleGOLD(rewardToken) is whitelisted for transfer and receive in the TempleGoldStakinthis token should be additionally tracked in the contract so we can calculate the maximum recoverable amount. Something like balanceOf(rewardToken) - totalRewardTokenMinted

Impact

The accidentially trasferred recoverTokenand rewardTokenare forever stuck in the contract.

It looks like the Impact is high because tokens are getting stuck. And the likelihood seems like a Medium because tokens have to be accidentially transferred. Hence the severity is High.

Tools Used

Manual Review

Recommendations

Allow recoverTokento recover rewardTokenand stakingToken but limit the amount which can be recovered.
Consider we have added a totalRewardAvailable storage variable in the TempleGoldStaking contract to track the actual rewardToken amount minted and available for reawards.

For example we can change the following functions in TempleGoldStakingin the following way:

function recoverToken(address _token, address _to, uint256 _amount) external override onlyElevatedAccess {
- if (_token == address(stakingToken) || _token == address(rewardToken ))
+ if ((_token == address(stakingToken) && amount > stakingToken.balanceOf(address(this) - totalSupply))
+ || (_token == address(rewardToken ) && amount > rewardToken.balanceOf(address(this) - totalRewardsAvailable)))
- { revert CommonEventsAndErrors.InvalidAmount(); }
+ { revert CommonEventsAndErrors.InvalidAmount(_token, _amount); }
IERC20(_token).safeTransfer(_to, _amount);
emit CommonEventsAndErrors.TokenRecovered(_to, _token, _amount);
}
function _getReward(address staker, address rewardsToAddress, uint256 index) internal {
uint256 amount = claimableRewards[staker][index];
if (amount > 0) {
claimableRewards[staker][index] = 0;
+ totalRewardsAvailable -= amount;
rewardToken.safeTransfer(rewardsToAddress, amount);
emit RewardPaid(staker, rewardsToAddress, index, amount);
}
}
function notifyDistribution(uint256 amount) external {
if (msg.sender != address(rewardToken)) { revert CommonEventsAndErrors.InvalidAccess(); }
/// @notice Temple Gold contract mints TGLD amount to contract before calling `notifyDistribution`
nextRewardAmount += amount;
+ totalRewardsAvailabe += amount;
emit GoldDistributionNotified(amount, block.timestamp);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

dianivanov Submitter
11 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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