Summary
TempleGoldStaking::setMigrator
is supposed to be called with an input parameter of a new staking contract. However, there are no checks that the address entered is a smart contract. In case of a mistake, an EOA could be set, which would allow the user to drain the whole staking contract with the usage of the migrateWithdraw
function.
function setMigrator(address _migrator) external override onlyElevatedAccess {
if (_migrator == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
migrator = _migrator;
emit MigratorSet(_migrator);
}
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;
}
Impact
The impact of this would be the drainage of the TempleGoldStaking
contract, stealing all of the users' stake tokens.
Tools Used
Manual review
Recommendations
Ensure that the migrator address is a smart contract address:
function setMigrator(address _migrator) external override onlyElevatedAccess {
if (_migrator == address(0)) { revert CommonEventsAndErrors.InvalidAddress(); }
+ if (_migrator.code.length == 0) { revert(); }
migrator = _migrator;
emit MigratorSet(_migrator);
}