0xAadi - Failure to Remove ERC20 Allowances on `rewardPool` Update in `StrategyPassiveManagerVelodrome`

[sherlock-admin2]

0xAadi

medium

Failure to Remove ERC20 Allowances on rewardPool Update in StrategyPassiveManagerVelodrome

Summary

The StrategyPassiveManagerVelodrome contract grants ERC20 token allowances to rewardPool. However, it fails to revoke these allowances when rewardPool is updated, creating a potential security risk.

Vulnerability Detail

The vulnerability arises from the following functions:

Granting Allowances

The _giveAllowances() function grants maximum allowances to rewardPool for the ERC20 tokens:

    function _giveAllowances() private {
        IERC20Metadata(output).forceApprove(unirouter, type(uint256).max);
        IERC20Metadata(output).forceApprove(rewardPool, type(uint256).max);
        IERC20Metadata(lpToken0).forceApprove(nftManager, type(uint256).max);
        IERC20Metadata(lpToken1).forceApprove(nftManager, type(uint256).max);
    }

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L827C5-L832C6

Updating unirouter

The rewardPool address can be updated via the setRewardPool function:

    function setRewardPool(address _rewardPool) external onlyOwner {
        rewardPool = _rewardPool;
        emit SetRewardPool(_rewardPool);
    }

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L776C5-L779C6

Currently, allowances are only removed by calling panic(). However, rewardPool can be changed at any time via the setRewardPool function without removing the previous allowances.

Impact

If rewardPool is updated via setRewardPool without removing the previous allowances, the old rewardPool contract retains the permissions to spend the protocol's tokens. This can lead to unintended token spending by the old rewardPool, which contradicts the protocol's intention after updating rewardPool.

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L776C5-L779C6

Tool used

Manual Review

Recommendation

Update setRewardPool in StrategyPassiveManagerVelodrome to remove all allowances before update rewardPool.

Duplicate of #3