search

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

5 stars 5 forks source link

EgisSecurity - StrategyPassiveManagerVelodrome.sol#setRewardPool() - When changing `rewardPool` allowances are kept and aren't give to the new `rewardPool` #86

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

EgisSecurity

medium

StrategyPassiveManagerVelodrome.sol#setRewardPool() - When changing rewardPool allowances are kept and aren't give to the new rewardPool

Summary

StrategyPassiveManagerVelodrome.sol#setRewardPool() - When changing rewardPool allowances are kept and aren't give to the new rewardPool

Vulnerability Detail

This issue is almost the same as 7.3.3 from this previous audit

When changing the rewardPool, we don't give allowances to the new rewardPool, this will be an issue when calling rewardPool.notifyRewardAmount as the function will use safeTransferFrom to attempt to pull the reward tokens out of StrategyPassiveManagerVelodrome, which will revert, since it doesn't have an allowance.

Also the old allowance isn't removed, which shouldn't be the case.

Impact

DoS of harvest and leftover allowance to old rewardPool

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

Mirror the logic inside setUnirouter

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

Duplicate of #3