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

Lack of Contract Verification in Migrator Functions of TempleGoldStaking Contract

maanvad3r

Summary

The TempleGoldStaking.sol contract has a function setMigrator() which allows a privileged user with onlyElevatedAccess to set the address of a new staking contract. In the same contract, the function migrateWithdraw() will be called after setMigrator() from the new staking contract address. However, in the setMigrator() function, there are no checks in place to determine if the address provided is actually a contract address. This means that it can accidentally be set to an externally owned account (EOA) address or any other address. While the function is protected by the onlyElevatedAccess modifier, this issue still presents a potential risk.

Vulnerability Details

The setMigrator() function allows setting a new migrator address, which is supposed to be the address of a new staking contract. However, the function does not check whether the provided address is a contract address. This can lead to a situation where an EOA address or any other addresses is set as the migrator.

Vulnerable Code

  • https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGoldStaking.sol#L135-L139

  • https://github.com/Cyfrin/2024-07-templegold/blob/main/protocol/contracts/templegold/TempleGoldStaking.sol#L163-L165

Impact

If an incorrect address, such as an EOA, is set as the migrator, it could lead to several issues:

  1. Security Risk: An EOA set as the migrator could execute unauthorized transactions, potentially leading to the loss of staked tokens.

  2. Operational Risk: The migration process could fail if the address is not a valid contract, disrupting the staking mechanism and user experience.

Tools Used

  • Manual Review

  • VS Code

Recommendations

Add check in the setMigrator() function to check if the address is a contract

function migrateWithdraw(address staker, uint256 index) external override onlyMigrator returns (uint256) {
require(staker != address(0), "Invalid address");
require(msg.sender.isContract(), "Migrator must be a contract");
StakeInfo storage _stakeInfo = _stakeInfos[staker][index];
uint256 stakerBalance = _stakeInfo.amount;
_withdrawFor(_stakeInfo, staker, msg.sender, index, _stakeInfo.amount, true, staker);
return stakerBalance;
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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