no - StrategyPassiveManagerVelodrome doesn't give ERC20 token allowances to new rewardPool when rewardPool is updated

[sherlock-admin3]

no

medium

StrategyPassiveManagerVelodrome doesn't give ERC20 token allowances to new rewardPool when rewardPool is updated

Summary

StrategyPassiveManagerVelodrome doesn't give ERC20 token allowances to new rewardPool when rewardPool is updated

Vulnerability Detail

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

We can see that rewardPool can be changed any time via the setRewardPool function. This allows the contract to enter a state where rewardPool is updated via setRewardPool but the ERC20 token approvals given to the old rewardPool are not removed. StrategyPassiveManagerVelodrome doesn't give ERC20 token allowances to new rewardPool when rewardPool is updated either. The new rewardPool have no approval, result in function _harvest() revert.

Impact

The old rewardPool contract will continue to have ERC20 token approvals. The new rewardPool have no approval, result in function _harvest() revert.

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L776C1-L779C6 https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L449

Tool used

Manual Review

Recommendation

function setRewardPool(address _rewardPool) external onlyOwner {
+        IERC20Metadata(output).forceApprove(rewardPool, 0);
        rewardPool = _rewardPool;
+         IERC20Metadata(output).forceApprove(rewardPool, type(uint256).max);
        emit SetRewardPool(_rewardPool);
    }

Duplicate of #3