search

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

5 stars 5 forks source link

BaldHeads - Withdraw Functionality Redeploys Liquidity In Non Calm Times Leading to Possible Financial Losses #91

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

BaldHeads

high

Withdraw Functionality Redeploys Liquidity In Non Calm Times Leading to Possible Financial Losses

Summary

The Withdraw function deploys liquidity to pool in non-calm times violating the invariant on the system.

Vulnerability Detail

In the veldrome strategy contract(StrategyPassiveManagerVelodrome), The deposit function 

    function deposit() external onlyCalmPeriods {
        _onlyVault();

        if (!initTicks) {
            _setTicks();
            initTicks = true;
        }

        // Add all liquidity
        _addLiquidity();

        (uint256 bal0, uint256 bal1) = balances();

        // TVL Balances after deposit
        emit TVL(bal0, bal1);
    }

has the onlyCalmPeriods modifier https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager-mohmoniem281/blob/1cc2325f43c8d7048b046cc595ce4b52c8d97061/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L117 which prevents deploying liquidity to velodrome during times where the price deviates with a certain threshold from the current upper and lower ticks. This modifier is also added to the setTicks() function https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager-mohmoniem281/blob/1cc2325f43c8d7048b046cc595ce4b52c8d97061/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L630 to prevent the system from adjusting the upper and lower tick range to ranges which may be inflated by malicious actions on the Veldrome pool. The withdraw function has the same effect on the system as it redeploys liquidity after performing the withdraw, however, without using the onlyCalmPeriods modifier.

Impact

This can lead to deploying liquidity in non favorable ranges on velodrome leading to many different scenarios including forced selling/swaps at inflated prices.

Code Snippet

Tool used

Manual Review

Recommendation

Add the onlycalmperiod modifier to the _addLiquidity function. this is the main entry point to adding liquidity to Veldrome and should be protected. Please note that there may be some side effects to this solution as users won't be able to withdraw their funds in non-calm periods (since withdraw deploys liquidity)

Duplicate of #74