search

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

5 stars 5 forks source link

0xDazai - Usage of `slot0` can be easly manipulated and lead to price manipulation #96

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

0xDazai

high

Usage of slot0 can be easly manipulated and lead to price manipulation

High

Summary

Usage of slot0 is dangerous because It is easy to manipulate.

Vulnerability Detail

StrategyPassiveManagerVelodrome.sol is using slot0 to calculate multiple variables in the contract.

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

    function currentTick() public view returns (int24 tick) {
        (,tick,,,,) = IVeloPool(pool).slot0();
    }

currentTick() function is returning the current tick of the pool using slot0 . The function is being called in isCalm() which must allow deposit/setTick actions when current price is within a certain deviation of twap.

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

    function isCalm() public view returns (bool) {
        int24 tick = currentTick();
        ....
        ....
   }

It is also called in _setTicks() which sets the tick positions for the main and alternative positions which is also used in four more main functions like unpause(), deposit(), moveTicks() , setPositionWidth() .

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

    function _setTicks() private onlyCalmPeriods {
        int24 tick = currentTick();
        ...
        ...
    }

The second function which is using slot0 is sqrtPrice() which returns the sqrtPriceX96 of the pool. https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L617-L619

    function sqrtPrice() public view returns (uint160 sqrtPriceX96) {
        (sqrtPriceX96,,,,,) = IVeloPool(pool).slot0();
    }

sqrtPrice() is used in _addLiquidity() https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L243-L261

    function _addLiquidity() private {
        ....
        ....

        uint160 sqrtprice = sqrtPrice();

        ....
        ....
     }

_checkAmounts() https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L402-L412

    function _checkAmounts(uint128 _liquidity, int24 _tickLower, int24 _tickUpper) private view returns (bool) {
        (uint256 amount0, uint256 amount1) = LiquidityAmounts.getAmountsForLiquidity(
      @>      sqrtPrice(),
            TickMath.getSqrtRatioAtTick(_tickLower),
            TickMath.getSqrtRatioAtTick(_tickUpper),
            _liquidity
        );

        ...
        ...

       }

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

    function balancesOfPool() public view returns (uint256 token0Bal, uint256 token1Bal, uint256 mainAmount0, uint256 mainAmount1, uint256 altAmount0, uint256 altAmount1) {
        uint160 sqrtPriceX96 = sqrtPrice();

        ...
        ...
        }

price() function which calculates the price of the pool https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/42ef5f0eac1bc954e888cf5bfb85cbf24c08ec76/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L608-L611 

    function price() public view returns (uint256 _price) {
        uint160 sqrtPriceX96 = sqrtPrice();
        _price = FullMath.mulDiv(uint256(sqrtPriceX96), SQRT_PRECISION, (2 ** 96)) ** 2;
    }

The usage of slot0 is throuought the whole contract. It is used for price calculation of the pool aslo for sqrtPriceX96, variables which are used on many places in the contract and are important to be correct. In case of manipulation of slot0 it can inflate the whole protocol and lead to loss of funds for the team and the users.

Impact

LP value can be manipulated to cause loss of funds for the protocol and the users.

Code Snippet

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

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

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

Tool used

Manual Review

Recommendation

To address this issue, avoid relying on slot0 and instead use TWAP.

Duplicate of #10