Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/libraries/PoolVariables.sol#L250-L253

Vulnerability details

Impact

In times of high market turbulence where price impact needs to be set above 50% in order to swap, the position would not be able to properly rebalance.

Proof of Concept

When rebalancing in TALOS, the protocol will eventually call UniswapV3Pool.swap.

https://github.com/Uniswap/v3-core/blob/main/contracts/UniswapV3Pool.sol#L595-L602
    /// @inheritdoc IUniswapV3PoolActions
    function swap(
        address recipient,
        bool zeroForOne,
        int256 amountSpecified,
>       uint160 sqrtPriceLimitX96,
        bytes calldata data
    ) external override noDelegateCall returns (int256 amount0, int256 amount1) {

Before calling swap, TALOS Poolvariables.sol will calculate the appropriate sqrtPriceLimitX96.

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/libraries/PoolVariables.sol#L250-L253
        // Calculate Price limit depending on price impact
        uint160 exactSqrtPriceImpact = (sqrtPriceX96 * (_strategy.priceImpactPercentage() / 2)) / GLOBAL_DIVISIONER;
        sqrtPriceLimitX96 = zeroForOne ? sqrtPriceX96 - exactSqrtPriceImpact : sqrtPriceX96 + exactSqrtPriceImpact;
    }

Focusing on the exactSqrtPriceImpact calculation, we see that the _strategy.priceImpactPercentage is divided by 2. Price impact percentage is set by the owner in TalosOptimizer.sol

    function setPriceImpact(uint24 _priceImpactPercentage) external onlyOwner {
        if (_priceImpactPercentage >= 1e6 || _priceImpactPercentage == 0) {
            revert PriceImpactPercentageInvalid();
        }
        priceImpactPercentage = _priceImpactPercentage;
    }

The price impact percentage cannot be set above 1e6 (which is 100%) and 0. Back to the exactSqrtPriceImpact calculation, because price impact percentage is divided by 2, price impact cannot be set above 50% when the original range is from 0-100%. This division effective restricts the range that the price impact percentage can go, which restricts the flexibility of sqrtPriceLimitX96 when swapping.

Tools Used

VSCode

Recommended Mitigation Steps

Recommend removing the division by 2 beside the priceImpactPercentage variable.

https://github.com/code-423n4/2023-05-maia/blob/main/src/talos/libraries/PoolVariables.sol#L250-L253
        // Calculate Price limit depending on price impact
-        uint160 exactSqrtPriceImpact = (sqrtPriceX96 * (_strategy.priceImpactPercentage() / 2)) / GLOBAL_DIVISIONER;
+        uint160 exactSqrtPriceImpact = (sqrtPriceX96 * (_strategy.priceImpactPercentage())) / GLOBAL_DIVISIONER;
        sqrtPriceLimitX96 = zeroForOne ? sqrtPriceX96 - exactSqrtPriceImpact : sqrtPriceX96 + exactSqrtPriceImpact;
    }

Assessed type

Context

code-423n4 / 2023-05-maia-findings

exactSqrtPriceImpact is unnecessarily divided by 2, which restricts the range of price impact percentage #822