recoverERC20 can be used by the owner as a backdoor to retrieve the rewardsToken

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L653-L662

Vulnerability details

Impact

The recoverERC20 () function is used to recover tokens sent to the contract by mistake. The contract requires one parameter token and can only be called by the owner. However, from the function code, the contract owner can use this function to recover any token including the rewardsToken used in the contract as there is no check to prevent this from happening from the code snippet shown below:

    function recoverERC20(address token) external onlyOwner returns(bool) {
        if(minAmountRewardToken[token] != 0) revert Errors.CannotRecoverToken();

        uint256 amount = IERC20(token).balanceOf(address(this));
        if(amount == 0) revert Errors.NullValue();
        IERC20(token).safeTransfer(owner(), amount);

        return true;
    }

As a result, users can lose all their rewards to a malicious or compromised owner.

Proof of Concept

https://github.com/code-423n4/2022-10-paladin/blob/d6d0c0e57ad80f15e9691086c9c7270d4ccfe0e6/contracts/WardenPledge.sol#L653-L662

Tools Used

Recommended Mitigation Steps

Consider implementing this function as follows:

    function recoverERC20(address token) external onlyOwner returns(bool) {
        require(token != rewardToken;

        if(minAmountRewardToken[token] != 0) revert Errors.CannotRecoverToken();

        uint256 amount = IERC20(token).balanceOf(address(this));
        if(amount == 0) revert Errors.NullValue();
        IERC20(token).safeTransfer(owner(), amount);

        return true;
    }

References

1. [WP-H29] StakingRewards.sol recoverERC20() can be used as a backdoor by the owner to retrieve rewardsToken 2022-02-concur-findings#210
2. StakingRewards.recoverERC20 allows owner to rug the rewardsToken 2022-02-concur-findings#69

[Kogaroshi]

Duplicate of #17

[c4-judge]

kirk-baird marked the issue as not a duplicate

[c4-judge]

kirk-baird marked the issue as duplicate

[c4-judge]

kirk-baird marked the issue as satisfactory

[c4-judge]

kirk-baird marked the issue as not a duplicate

[c4-judge]

kirk-baird marked the issue as duplicate of #17

[c4-judge]

Simon-Busch marked the issue as duplicate of #68