search

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

5 stars 5 forks source link

0xShoonya - `twap()` will show incorrect price for negative ticks cause it doesn't round up for negative ticks #107

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

0xShoonya

medium

twap() will show incorrect price for negative ticks cause it doesn't round up for negative ticks

Summary

The twap() function in StrategyPassiveManagerVelodrome.sol incorrectly calculates the price for negative ticks, failing to round down as needed. This miscalculation can lead to price manipulation and arbitrage opportunities. To fix this, adjust the function to round down negative ticks correctly.

Vulnerability Detail

    function twap() public view returns (int56 twapTick) {
        uint32[] memory secondsAgo = new uint32[](2);
        secondsAgo[0] = uint32(twapInterval);
        secondsAgo[1] = 0;

        (int56[] memory tickCuml,) = IVeloPool(pool).observe(secondsAgo);
        twapTick = (tickCuml[1] - tickCuml[0]) / int32(twapInterval);
    }

twap()function in StrategyPassiveManagerVelodrome.sol is based on the uniswap's OracleLibrary.sol::consult() function. It uses IVeloPool(pool).observe to get tickCumul array which is then used to calculate int56 twapTick.

The problem is that in case if int56(tickCuml[1] - tickCuml[0]) is negative, then the tick should be rounded down as it's done in the uniswap library.

Impact

As a result, in case if int56(tickCuml[1] - tickCuml[0]) is negative and (tickCuml[1] - tickCuml[0]) % secondsAgo != 0, then returned tick will be bigger then it should be, which opens possibility for some price manipulations and arbitrage opportunities.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add these lines in the twap() function

int56 tickCumlDelta = tickCuml[1] - tickCuml[0];
timeWeightedTick = int24(tickCumlsDelta / secondsAgo);
if (int56(tickCuml[1] - tickCuml[0]) < 0 && ((tickCuml[1] - tickCuml[0]) % secondsAgo != 0) { timeWeightedTick -- };

Duplicate of #95

sherlock-admin3 commented 2 months ago

2 comment(s) were left on this issue during the judging contest.

WildSniper commented:

borderLine medium low

WildSniper commented:

seems like optimizing the protocol other than a valid bug and taking into consideration how the main position is two sided not one sided