Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/pool.rs#L75-L87

Vulnerability details

Impact

Malicious actor can create a position with tickLower==tickUpper and leverage that position to artificially increase gross liquidity to the maximum liquidity of that tick, consequently, preventing normal users from adding an actual liquidity on that tick.
Malicous actor can create a position with tickLower > tickUpper and makes the pool effectively short on tokens compared to its net liquidity. (token is added less than delta liquidity added)

Description

Users must specify a tickLower and tickUpper for their position when creating a new position.
tickLower and tickUpper represent the price range at which users want to provide liquidity.

Here is the function, create_position, where the logic for creating a new position resides.

// File: pkg/seawater/src/pool.rs
pub fn create_position(&mut self, id: U256, low: i32, up: i32) -> Result<(), Revert> {
    assert_or!(self.enabled.get(), Error::PoolDisabled);
    let spacing = self.tick_spacing.get().sys() as i32;
    assert_or!(low % spacing == 0, Error::InvalidTickSpacing);
    assert_or!(up % spacing == 0, Error::InvalidTickSpacing);
    let spacing: u8 = spacing.try_into().map_err(|_| Error::InvalidTickSpacing)?;
    let min_tick = tick_math::get_min_tick(spacing);
    let max_tick = tick_math::get_max_tick(spacing);
    assert_or!(low >= min_tick && low <= max_tick, Error::InvalidTick);
    assert_or!(up >= min_tick && up <= max_tick, Error::InvalidTick);
    self.positions.new(id, low, up);
    Ok(())
}

It validates that:

specified tick must be compatible with the pool's tick spacing
specified tick must be within range of minimum and maxmimum tick

This almost resembles how UniswapV3 validates the input tick. One validation it lacks is: specified tickLower must be less than tickUpper.

See: UniswapV3Pool.checkTicks

This allows users to create a position with:

tickLower == tickUpper
tickLower > tickUpper

Unfortunately, all functions/features related to position's liquidity operate on the assumption that tickLower is always less than tickUpper.
Therefore, creating a position with such settings could result in an unexpected outcome.

Let's explore each path for possible outcome.

tickLower == tickUpper

First, let's revisit some formulas used to calculate the required amount of token0 and token given liquidity_delta.

$P < P{lower}$
$\Delta \text{token0} = \Delta L \left( \frac{\sqrt{P{upper}}-\sqrt{P{lower}}}{\sqrt{P}\sqrt{P{upper}}} \right)$
$\Delta \text{token1} = 0$

$P{lower} < P < P{upper}$
$\Delta \text{token0} = \Delta L \left( \frac{\sqrt{P{upper}}-\sqrt{P}}{\sqrt{P}\sqrt{P{upper}}} \right)$
$\Delta \text{token1} = \Delta L \left( \sqrt{P} - \sqrt{P_{lower}} \right)$

$P > P{upper}$
$\Delta \text{token0} = 0$
$\Delta \text{token1} = \Delta L \left( \sqrt{P{upper}} - \sqrt{P_{lower}} \right)$

For more information, please see: https://blog.uniswap.org/uniswap-v3-math-primer-2

If tickLower and tickUpper are the same value, then P_lower is the same value as P_upper.

As a result, for all conditions of current price, the required token0 and token1 always yield 0 for any given liquidity.

This means that gross liquidity can be added with arbitrary large value of liquidity delta and use exactly zero amount of both token0 and token1.

Here are some relevant implementations for providing/removing liquidity to a position.

// File: pkg/seawater/pool.rs
pub fn update_position(&mut self, id: U256, delta: i128) -> Result<(I256, I256), Revert> {
    // ... snipped ...
    if delta != 0 {
        flipped_lower = self.ticks.update(
            lower,
            cur_tick,
            delta,
            &fee_growth_global_0,
            &fee_growth_global_1,
            false,
            max_liquidity_per_tick,
        )?;

        flipped_upper = self.ticks.update(
            upper,
            cur_tick,
            delta,
            &fee_growth_global_0,
            &fee_growth_global_1,
            true,
            max_liquidity_per_tick,
        )?;
    // ... snipped ...

// File: pkg/seawater/tick.rs
pub fn update(
    &mut self,
    tick: i32, // lower/upper
    cur_amm_tick: i32,
    liquidity_delta: i128,
    fee_growth_global_0: &U256,
    fee_growth_global_1: &U256,
    upper: bool,
    max_liquidity: u128,
) -> Result<bool, Error> {
    let mut info = self.ticks.setter(tick);

    let liquidity_gross_before = info.liquidity_gross.get().sys();
    let liquidity_gross_after =
        liquidity_math::add_delta(liquidity_gross_before, liquidity_delta)?;

    if liquidity_gross_after > max_liquidity {
        return Err(Error::LiquidityTooHigh);
    }
    // ... snipped ...
    info.liquidity_gross.set(U128::lib(&liquidity_gross_after));
    // ... snipped ...

We can see here that liquidity delta is added to gross liquidity of both lower and upper tick and if it reaches the limit, it throws an error.

All in all, a malicious actor can leverage position with tickLower==tickUpper to artifically increase gross liquidity of each tick without the need to provide any amount of token0 and token1. It can be increased to the point of maximum liquidity allowed (or maximum of u128 type).

Consequently, it will prevent other users from adding an actual liquidity to the given tick as their transaction will fail from Err(Error::LiquidityTooHigh).

tickLower > tickUpper

When liquidity is added on a position with tickLower > tickUpper, the protocol interprets the active range of this position as:

[minTick, tickUpper] and [tickLower, maxTick]

An inverse range, if you will.
If the tickLower is 10 and tickUpper is 1, the protocol adds delta liquidity on tick.10 and removes delta liquidity on tick.1. Hence, inverse range.

However, it incorrectly calculates the required amount0 and amount1.

For example, if the current tick of the pool is less than tickLower, it will calculate amount0 using this formula:
$\Delta \text{token0} = \Delta L \left( \frac{\sqrt{P{upper}}-\sqrt{P{lower}}}{\sqrt{P}\sqrt{P_{upper}}} \right)$

In this case, P_upper is corresponding to tickUpper, P_lower is corresponding to tickLower and thanks to how _get_amount_x_delta works, if P_lower is greater than P_upper, it automatically switches places.

All in all, it calculates required amount0 for the position of range [tickUpper, tickLower] which is smaller than it should be as the protocol actually interprets this position's range as [tickLower, maxTick].

This creates an accounting problem for the pool as it could cause a mismatch between the total liquidity and the total balance of tokens in the pool.
It might interpret that on a given tick that there is 100 token0 available while only 10 token0 is added to the pool.

get_amount_0/1_delta where it swaps P_lower and P_upper:

pub fn _get_amount_0_delta(
    mut sqrt_ratio_a_x_96: U256,
    mut sqrt_ratio_b_x_96: U256,
    liquidity: u128,
    round_up: bool,
) -> Result<U256, Error> {
    if sqrt_ratio_a_x_96 > sqrt_ratio_b_x_96 {
        (sqrt_ratio_a_x_96, sqrt_ratio_b_x_96) = (sqrt_ratio_b_x_96, sqrt_ratio_a_x_96)
    };
    // ...snipped...
}

pub fn _get_amount_1_delta(
    mut sqrt_ratio_a_x_96: U256,
    mut sqrt_ratio_b_x_96: U256,
    liquidity: u128,
    round_up: bool,
) -> Result<U256, Error> {
    if sqrt_ratio_a_x_96 > sqrt_ratio_b_x_96 {
        (sqrt_ratio_a_x_96, sqrt_ratio_b_x_96) = (sqrt_ratio_b_x_96, sqrt_ratio_a_x_96)
    };
    // ...snipped...
}

Proof-of-Concept

The PoC shows the scenario of tickLower==tickUpper mentioned in Description section.

Attacker creates a position with tickLower==tickUpper and artificially increase gross liquidity to max_liqudity.
Normal user attempts to add liquidity to that tick but fail from Liquidity Too High error.

Steps

Add the following test in pkg/seawater/tests/lib.rs
Run cargo test --package seawater --features testing test_ticklower_equals_tickupper -- --nocapture

Observe that the final update_position fails from Liquidity higher than max

#[test]
fn test_ticklower_equals_tickupper() -> Result<(), Vec<u8>> {
test_utils::with_storage::<_, Pools, _>(
    Some(address!("feb6034fc7df27df18a3a6bad5fb94c0d3dcb6d5").into_array()),
    None, // slots map
    None, // caller erc20 balances
    None, // amm erc20 balances
    |contract| {
        // Create the storage
        contract.ctor(msg::sender(), Address::ZERO, Address::ZERO)?;

        // Setting up the pool, max_liquidity = 1e18
        let token_addr = address!("97392C28f02AF38ac2aC41AF61297FA2b269C3DE");
        let maximum_liquidity: i128 = 1000000000000000000; // 1e18
        contract.create_pool_D650_E2_D0(
            token_addr,
            test_utils::encode_sqrt_price(50, 1),
            0,
            1,
            maximum_liquidity.try_into().unwrap(),
        )?;
        // Enable the pool for trading
        contract.enable_pool_579_D_A658(token_addr, true)?;

        // Create a position with tickLower == tickUpper
        let lower_tick = test_utils::encode_tick(50);
        let upper_tick = test_utils::encode_tick(50);

        contract.mint_position_B_C5_B086_D(token_addr, lower_tick, upper_tick)?;
        let position_id = contract
            .next_position_id
            .clone()
            .checked_sub(U256::one())
            .unwrap();

        /**
        Add liquidity, use half of maximum_liquidity because gross liquidity is added twice for lower and upper tick
        */
        let liquidity_delta = maximum_liquidity/2;
        contract.update_position_C_7_F_1_F_740(token_addr, position_id, liquidity_delta)?;

        // Create a new position with tickLower equals to the prev position, so that it attempts to add liquidity to the same tick
        contract.mint_position_B_C5_B086_D(token_addr, lower_tick, upper_tick+1)?;
        let next_position_id = contract
            .next_position_id
            .clone()
            .checked_sub(U256::one())
            .unwrap();

        // Try adding liquidity but fails because the maximum liquidity is already reached
        let res = contract.update_position_C_7_F_1_F_740(token_addr, next_position_id, 100).expect_err("It should give error");
        println!("{:?}", String::from_utf8(res));
        Ok(())
    },
)
}

Recommended Mitigations

Add another assert statement to ensure that tickLower is less than tickUpper

pub fn create_position(&mut self, id: U256, low: i32, up: i32) -> Result<(), Revert> {
assert_or!(self.enabled.get(), Error::PoolDisabled);
let spacing = self.tick_spacing.get().sys() as i32;
assert_or!(low % spacing == 0, Error::InvalidTickSpacing);
assert_or!(up % spacing == 0, Error::InvalidTickSpacing);
let spacing: u8 = spacing.try_into().map_err(|_| Error::InvalidTickSpacing)?;
let min_tick = tick_math::get_min_tick(spacing);
let max_tick = tick_math::get_max_tick(spacing);
assert_or!(low >= min_tick && low <= max_tick, Error::InvalidTick);
assert_or!(up >= min_tick && up <= max_tick, Error::InvalidTick);
assert_or!(low < up, Error::InvalidTick); // <-- add this validation
self.positions.new(id, low, up);
Ok(())
}

Assessed type

Context

code-423n4 / 2024-08-superposition-findings

Insufficient validation of tickLower and tickUpper when creating a new position #38