Feature to recover stuck tokens is too permissive and could be used to remove CVX tokens

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategyCore.sol#L215-L220

Vulnerability details

Summary

The VotingStrategyCore contract contains a function to allow the owner of the protocol to withdraw any ERC20 token, including the CVX token, which is core to the contract's functionality.

Impact

The withdrawStuckTokens() function present in the VotingStrategyCore contract allows an admin to recover any "stuck" ERC20 token:

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategyCore.sol#L215-L220

215:     function withdrawStuckTokens(address _token) public onlyOwner {
216:         IERC20(_token).transfer(
217:             msg.sender,
218:             IERC20(_token).balanceOf(address(this))
219:         );
220:     }

While this seems harmless and would allow the protocol to correct unexpected mistakes, it is important to note that the core of the Votium strategy is composed of CVX tokens. These tokens shouldn't be allowed to be recovered or withdrawn from the contract. Even if it is an owner controlled function, it is important to keep the transparency here.

Recommendation

Validate the token address is different from the CVX contract, to ensure that CVX tokens cannot be unintentionally withdrawn using this function.

    function withdrawStuckTokens(address _token) public onlyOwner {
+       require(_token != CVX_ADDRESS);
        IERC20(_token).transfer(
            msg.sender,
            IERC20(_token).balanceOf(address(this))
        );
    }

Assessed type

Rug-Pull

[c4-judge]

0xleastwood marked the issue as primary issue

[c4-judge]

0xleastwood marked the issue as selected for report

[c4-sponsor]

elmutt (sponsor) confirmed

[0xleastwood]

Downgrading this to QA as it is sort of raised in the automated findings, but I do believe the suggestion is worthwhile to implement.

[c4-judge]

0xleastwood marked the issue as not selected for report

[c4-judge]

0xleastwood changed the severity to QA (Quality Assurance)