aman - `positionAlt` ticks will not be updated if `amount0==bal1`

[sherlock-admin2]

aman

medium

positionAlt ticks will not be updated if amount0==bal1

Summary

The protocol allows owner to update the tick to add liquidity into pool. To set the tick of alternative position the protocol set it ticks on basis of token which values is more then other one. How ever it miss an edge case due to which the ticks for alternative position will not be updated.

Vulnerability Detail

To set or update the tick the owner of strategy will call setPositionWidth. This function will claim the current earning , remove the liquidity from pool , update the width of position , set the ticks and add liquidity. The Issue here is in _setTicks which calls _setAltTick with latest width . The implementation is given below:

function _setTicks() private onlyCalmPeriods {
        int24 tick = currentTick();
        int24 distance = _tickDistance();
        int24 width = positionWidth * distance; // @audit : not good may be overflow

        _setMainTick(tick, distance, width);
        _setAltTick(tick, distance, width);
    }

First we change the main position ticks and after that we change the ticks for alternative position. lets have a look on _setAltTick.

649:     function _setAltTick(int24 tick, int24 distance, int24 width) private {
650:         (uint256 bal0, uint256 bal1) = balancesOfThis();
651: 
652:         // We calculate how much token0 we have in the price of token1. 
653:         uint256 amount0;
654: 
655:         if (bal0 > 0) {
656:             amount0 = bal0 * price() / PRECISION;
657:         }
658: 
659:         // We set the alternative position based on the token that has the most value available. 
660:         // @audit : what if both are same??
661:         if (amount0 < bal1) {
662:             (positionAlt.tickLower, ) = TickUtils.baseTicks(
663:                 tick,
664:                 width,
665:                 distance
666:             );
667: 
668:             (positionAlt.tickUpper, ) = TickUtils.baseTicks(
669:                 tick,
670:                 distance,
671:                 distance
672:             ); 
673:         } else if (bal1 < amount0) {
674:             (, positionAlt.tickLower) = TickUtils.baseTicks(
675:                 tick,
676:                 distance,
677:                 distance
678:             );
679: 
680:             (, positionAlt.tickUpper) = TickUtils.baseTicks(
681:                 tick,
682:                 width,
683:                 distance
684:             ); 
685:         }
686:     }
687: 

lets assume bal0 is not 0. At line 656 we calculate the price for amount0 and in subsequent steps we check if amount0>bal1 then we update the tick on basis of tokne0 otherwise on token1. However there is chance if both amount0==bal1 the position will not be updated and keep operating on older ticks which the owner of strategy does not want to use or not that profitable.

Impact

The alternative position ticks will not updated and result in lose of profit or unintended behavior which is not required by the owner of strategy i.e using the older ticks.

Code Snippet

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L415C14-L415C23 https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L649-L685

Tool used

Manual Review

Recommendation

Replace < or >check with <= or >= in either condition :

+        if (amount0 <= bal1) {

Duplicate of #38

[amankakar]

Escalate How can this be borderline low As there is lose of funds due to using wrong limits for alternative position.

Its a low, this cause code bloat which could make the contract too large and at most would only last until there is another rebalance call. Won't fix.

There is no 100% surety that in second call this case would not occur again.

[sherlock-admin3]

Escalate How can this be borderline low As there is lose of funds due to using wrong limits for alternative position.

Its a low, this cause code bloat which could make the contract too large and at most would only last until there is another rebalance call. Won't fix.

There is no 100% surety that in second call this case would not occur again.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.