Skip to content
This repository has been archived by the owner on Nov 5, 2023. It is now read-only.

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

Closed
sherlock-admin opened this issue Apr 30, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Apr 30, 2023

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

@github-actions github-actions bot added the Medium A valid Medium severity issue label May 3, 2023
@hrishibhat hrishibhat added the Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label label May 19, 2023
@sherlock-admin sherlock-admin added High A valid High severity issue Reward A payout will be made for this issue and removed Medium A valid Medium severity issue labels May 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants