search

sherlock-audit / 2024-06-makerdao-endgame-judging

1 stars 1 forks source link

bareli - StakingRewards.recoverERC20 allows owner to rug the rewardsToken #53

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

bareli

Medium

StakingRewards.recoverERC20 allows owner to rug the rewardsToken

Summary

StakingRewards.recoverERC20 rightfully checks against the stakingToken being sweeped away. However, there's no check against the rewardsToken which over time will sit in this contract.

This is the case of an admin privilege, which allows the owner to sweep the rewards tokens, perhaps as a way to rug depositors.

Vulnerability Detail

function recoverERC20(address tokenAddress, uint256 tokenAmount) external onlyOwner { require(tokenAddress != address(stakingToken), "Cannot withdraw the staking token"); IERC20(tokenAddress).safeTransfer(owner, tokenAmount); emit Recovered(tokenAddress, tokenAmount); }

Impact

Allows owner to rug the rewardsToken.

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/endgame-toolkit/src/synthetix/StakingRewards.sol#L166C4-L170C6

Tool used

Manual Review

Recommendation

require( tokenAddress != address(rewardsToken), "Cannot withdraw the rewards token" );

Duplicate of #41

telome commented 1 month ago

The owner is assumed fully trusted and non malicious. From the competition rules: "Even when possible, governance is assumed to not confiscate/manipulate specific user funds/positions without a good reason. This means that reports that claim that governance can take specific users funds are not considered issues."