sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

0x52 - IchiSpell applies slippage to sqrtPrice which is wrong and leads to unpredictable slippage #131

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0x52

medium

IchiSpell applies slippage to sqrtPrice which is wrong and leads to unpredictable slippage

Summary

UniswapV3 uses the sqrt of the price rather than the price itself, but slippage is applied directly to the sqrt of the price which leads to unpredictable prices.

Vulnerability Detail

IchiSpell.sol#L211-L222

        uint160 deltaSqrt = (param.sqrtRatioLimit *
            uint160(param.sellSlippage)) / uint160(Constants.DENOMINATOR);
        SWAP_POOL.swap(
            address(this),
            // if withdraw token is Token0, then swap token1 -> token0 (false)
            !isTokenA,
            amountToSwap.toInt256(),
            isTokenA
                ? param.sqrtRatioLimit + deltaSqrt
                : param.sqrtRatioLimit - deltaSqrt, // slippaged price cap
            abi.encode(address(this))
        );

We see above that the sell slippage is applied to the sqrtPrice instead of the actual price. This means that applied slippage limits are not applied correctly and can be much larger than intended.

Example: The protocol wants to limit slippage to 10%. This limit is applied to the sqrt price so if the sqrt price is 10 (price = 100) then it will apply 10% to that making it 9. Now to get the true price we need to square the price which gives us a price of 81. This translates to a true slippage limit of 19% rather than 10%.

Impact

Slippage limits are much larger than intended

Code Snippet

IchiSpell.sol#L181-L236

Tool used

Manual Review

Recommendation

Apply slippage requirement on price not sqrt price

Duplicate of #132