sherlock-audit / 2024-05-beefy-cowcentrated-liquidity-manager-judging

5 stars 5 forks source link

jasonxiale - `StrategyPassiveManagerVelodrome.harvest` will revert after calling `StrategyPassiveManagerVelodrome.setRewardPool` #108

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

jasonxiale

medium

StrategyPassiveManagerVelodrome.harvest will revert after calling StrategyPassiveManagerVelodrome.setRewardPool

Summary

After calling StrategyPassiveManagerVelodrome.setRewardPool, the StrategyPassiveManagerVelodrome doesn't set allowance for rewardPool. Whthout the allowance, IRewardPool(rewardPool).notifyRewardAmount will revert.

Vulnerability Detail

After calling StrategyPassiveManagerVelodrome.setRewardPool, the allowance doesn't clear for the old rewardPool, and more important, the function doesn't set allowance for new rewardPool.

776     function setRewardPool(address _rewardPool) external onlyOwner {
777         rewardPool = _rewardPool;
778         emit SetRewardPool(_rewardPool);
779     }

Without the allowance, IRewardPool(rewardPool).notifyRewardAmount will revert because accordining to rewardPool in test

    function notifyRewardAmount(
        address _reward,
        uint256 _amount,
        uint256 _duration
    ) external onlyManager update(address(0)) {
...
        IERC20Upgradeable(_reward).safeTransferFrom(msg.sender, address(this), _amount); <<<--- function will revert because of lacking of allowance
...
    }

Impact

StrategyPassiveManagerVelodrome.harvest will revert.

Code Snippet

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

Tool used

Manual Review

Recommendation

diff --git a/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol b/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol
index 1340f25..202089a 100644
--- a/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol
+++ b/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol
@@ -774,7 +774,9 @@ contract StrategyPassiveManagerVelodrome is StratFeeManagerInitializable, IStrat
      * @param _rewardPool The new reward pool address.
      */
     function setRewardPool(address _rewardPool) external onlyOwner {
+        _removeAllowances();
         rewardPool = _rewardPool;
+        _giveAllowances();
         emit SetRewardPool(_rewardPool);
     }

@@ -842,4 +844,4 @@ contract StrategyPassiveManagerVelodrome is StratFeeManagerInitializable, IStrat
     function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
         return this.onERC721Received.selector;
     }
-}
\ No newline at end of file
+}

Duplicate of #3