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

`TempleGoldStaking.migrateWithdraw()` potentially reverts due to insufficient reward token balance

merlinboii

Summary

The TempleGoldStaking.migrateWithdraw() function forces the withdrawal of the FULL staking amount and the claim of ALL rewards.

However, the forced claiming of rewards can cause a reversion, preventing some stakers from being migrated.

Vulnerability Details

The TempleGoldStaking.migrateWithdraw() function forces the withdrawal of the full staking amount and the claiming of all rewards for the given staker.

The claimRewards flag is always set to TRUE, which means that rewards will be forced to be claimed in every call of TempleGoldStaking.migrateWithdraw().

However, the reward token balance of the staking contract can be less than the total claimable amount from all stakers, resulting in the inability to migrate for some stakers.

//Location: TempleGoldStaking.sol
​
function migrateWithdraw(address staker, uint256 index) external override onlyMigrator returns (uint256) {
if (staker == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
StakeInfo storage _stakeInfo = _stakeInfos[staker][index];
uint256 stakerBalance = _stakeInfo.amount;
@> _withdrawFor(_stakeInfo, staker, msg.sender, index, _stakeInfo.amount, true, staker);
return stakerBalance;
}
//Location: TempleGoldStaking.sol
​
function _withdrawFor(
StakeInfo storage stakeInfo,
address staker,
address toAddress,
uint256 stakeIndex,
uint256 amount,
@> bool claimRewards,
address rewardsToAddress
) internal updateReward(staker, stakeIndex) {
if (amount == 0) { revert CommonEventsAndErrors.ExpectedNonZero(); }
// snipped
​
if (claimRewards) {
// can call internal because user reward already updated
@> _getReward(staker, rewardsToAddress, stakeIndex);
}
}
​
// snipped
​
function _getReward(address staker, address rewardsToAddress, uint256 index) internal {
uint256 amount = claimableRewards[staker][index];
if (amount > 0) {
claimableRewards[staker][index] = 0;
@> rewardToken.safeTransfer(rewardsToAddress, amount);
emit RewardPaid(staker, rewardsToAddress, index, amount);
}
}

Impact

Some stakers may be unable to complete the migration process due to insufficient reward token balances. This can leave their funds locked in the old contract.

Proof of Concept

Setup

  • Put the snippet below into the protocol test suite: test/forge/templegold/TempleGoldStaking.t.sol

  • Run test: forge test --mt test_unable_to_migrate -vvv

Working Test Case

function test_unable_to_migrate() public {
// SET UP: START
address migrator = makeAddr("migrator");
uint32 _vestingPeriod = 16 weeks;
​
uint256 stakeAmount = 100 ether;
ITempleGoldStaking.Reward memory rewardData;
​
//tracking vars of total reward distribution to the TempleGoldStaking contract
uint256 TGLDdistribution;
uint256 stakingBalanceBefore;
uint256 stakingBalanceAfter;
​
{
vm.startPrank(executor);
staking.setMigrator(migrator);
_setVestingPeriod(_vestingPeriod);
_setRewardDuration(_vestingPeriod);
_setVestingFactor(templeGold);
​
deal(address(templeToken), alice, 1000 ether, true);
deal(address(templeToken), bob, 1000 ether, true);
vm.stopPrank();
}
// SET UP: END
{
skip(5 days);
// Alice 1st stake
vm.startPrank(alice);
_approve(address(templeToken), address(staking), type(uint).max);
stakingBalanceBefore = templeGold.balanceOf(address(staking));
staking.stake(stakeAmount);
stakingBalanceAfter = templeGold.balanceOf(address(staking));
TGLDdistribution += stakingBalanceAfter - stakingBalanceBefore;
vm.stopPrank();
​
skip(10 days);
// Distribution #1
stakingBalanceBefore = templeGold.balanceOf(address(staking));
staking.distributeRewards();
stakingBalanceAfter = templeGold.balanceOf(address(staking));
TGLDdistribution += stakingBalanceAfter - stakingBalanceBefore;
​
}
​
{
skip(100 days); // Make sure that it is end of Temple Gold Vestion Period
stakingBalanceBefore = templeGold.balanceOf(address(staking));
staking.distributeRewards();
stakingBalanceAfter = templeGold.balanceOf(address(staking));
TGLDdistribution += stakingBalanceAfter - stakingBalanceBefore;
​
// Bob 1st stake after vesting end
vm.startPrank(bob);
_approve(address(templeToken), address(staking), type(uint).max);
stakingBalanceBefore = templeGold.balanceOf(address(staking));
staking.stake(stakeAmount);
stakingBalanceAfter = templeGold.balanceOf(address(staking));
TGLDdistribution += stakingBalanceAfter - stakingBalanceBefore;
vm.stopPrank();
}
uint256 rewardPerToken;
uint256 _vestingRate;
​
{
rewardData = staking.getRewardData();
vm.warp(rewardData.periodFinish);
​
// Attemping to MIGRATE
vm.startPrank(migrator);
// Attemping to migrate: Bob (work)
rewardPerToken = staking.rewardPerToken();
_vestingRate = _getVestingRate(bob, 1);
uint256 bobEarned = _getEarned(stakeAmount, rewardPerToken, 0, _vestingRate);
vm.expectEmit(address(staking));
emit RewardPaid(address(bob), address(bob), 1, bobEarned);
staking.migrateWithdraw(bob, 1);
​
// Attemping to migrate: Alice (revert as insufficient rewards)
rewardPerToken = staking.rewardPerToken();
_vestingRate = _getVestingRate(alice, 1);
uint256 aliceEarned = _getEarned(stakeAmount, rewardPerToken, 0, _vestingRate);
vm.expectRevert(abi.encodeWithSelector(IERC20Errors.ERC20InsufficientBalance.selector, address(staking), templeGold.balanceOf(address(staking)), aliceEarned));
staking.migrateWithdraw(alice, 1);
vm.stopPrank();
​
console.log("bobEarned: %s", bobEarned);
console.log("aliceEarned: %s\n", aliceEarned);
console.log("Total Earned: %s", bobEarned + aliceEarned);
console.log("Does Total Earned > TGLDdistribution ?: %s", (bobEarned + aliceEarned) > TGLDdistribution);
}
​
{
// Prove of the vesting is end and maximum distribution is reach
console.log("TGLDdistribution for TemplGoldStaking: %s\n", TGLDdistribution);
​
ITempleGold.DistributionParams memory params = templeGold.getDistributionParameters();
uint256 MAX_DistributionForTempleGoldStaking = templeGold.MAX_SUPPLY() * params.staking / templeGold.DISTRIBUTION_DIVISOR();
assertEq(TGLDdistribution, MAX_DistributionForTempleGoldStaking);
}
}

Results of running the test:

Ran 1 test for test/forge/templegold/TempleGoldStaking.t.sol:TempleGoldStakingTest
[PASS] test_unable_to_migrate() (gas: 1286931)
Logs:
bobEarned: 170089285714285714282252800
aliceEarned: 170089285714285714282252800
​
Total Earned: 340178571428571428564505600
Does Total Earned > TGLDdistribution ?: true
TGLDdistribution for TemplGoldStaking: 300000000000000000000000000
​
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 772.06ms (4.59ms CPU time)
​
Ran 1 test suite in 958.28ms (772.06ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

  • Foundry

  • Manual review

Recommendations

  • Allow flexibility in the claiming reward flag via TempleGoldStaking.migrateWithdraw() to avoid the issue of insufficient rewards causing a reversion.

  • Update TempleGoldStaking._getReward() to avoid claiming an amount that exceeds the available balance.

    • (Optional approach) Allow users to claim their rewards up to the reward balance limit. However, this approach requires careful handling to ensure the correct claimableRewards are managed properly.

//Location: TempleGoldStaking.sol
​
- function migrateWithdraw(address staker, uint256 index) external override onlyMigrator returns (uint256) {
+ function migrateWithdraw(address staker, uint256 index, bool claim) external override onlyMigrator returns (uin
if (staker == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
StakeInfo storage _stakeInfo = _stakeInfos[staker][index];
uint256 stakerBalance = _stakeInfo.amount;
- _withdrawFor(_stakeInfo, staker, msg.sender, index, _stakeInfo.amount, true, staker);
+ _withdrawFor(_stakeInfo, staker, msg.sender, index, _stakeInfo.amount, claim, staker);
return stakerBalance;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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