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

breach in pause functionality as users can migrate to a new staking contract to vest when the current one is paused.

sancybars

Summary

The `migrateWithdraw` function allows users to migrate their staked tokens to a new staking contract even when the current contract is paused. This behavior undermines the pause mechanism, as users can bypass the pause restriction and continue their staking activities in a new contract.

Vulnerability Details

The `migrateWithdraw` function enables users to withdraw their staked tokens and migrate them to a new staking contract specified by the `migrator`. This function can be called regardless of whether the current contract is paused.

https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol%2Fcontracts%2Ftemplegold%2FTempleGoldStaking.sol#L168-L174

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

Issue

Allowing `migrateWithdraw` to be called during the paused state defeats the purpose of the pause functionality. Users can bypass the pause by migrating to a new contract and continuing their staking activities, rendering the pause ineffective.

Tools Used

Manual review

Recommendations

To ensure the pause functionality is effective, add the `whenNotPaused` modifier to the `migrateWithdraw` function.

function migrateWithdraw(address staker, uint256 index) external override onlyMigrator whenNotPaused 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;
​
}
​

Adding the `whenNotPaused` modifier ensures that the `migrateWithdraw` function can only be called when the contract is not paused.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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