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

0 stars 0 forks source link

Missing Tick Range Validation in create_position Function #189

Closed c4-bot-9 closed 1 month ago

c4-bot-9 commented 1 month ago

Lines of code

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

Vulnerability details

In the create_position function, there is no validation to check whether the provided lower tick is less than the upper tick, which is a crucial condition for ensuring proper tick range management.

When this validation is missing, it becomes possible for users to provide tick ranges where the lower tick is greater than the upper tick, leading to unexpected behaviors.

@>> 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(())
}

## Impact

The lack of validation for tick range order introduces potential inconsistencies in liquidity positions. When the ticks are inverted, the position behaves differently, affecting the token amounts that users must provide to establish or update their position.
This can also have some other issues too.

## Proof of Concept
```rust
pub fn update_position(&mut self, id: U256, delta: i128) -> Result<(I256, I256), Revert> {
        // the pool must be enabled
        assert_or!(self.enabled.get(), Error::PoolDisabled);

        let position = self.positions.positions.get(id);
        let lower = position.lower.get().sys();
        let upper = position.upper.get().sys();

        // update the ticks
        let cur_tick = self.cur_tick.get().sys();
        ...

        if delta != 0 {
            flipped_lower = self.ticks.update(
@>>             lower,
                cur_tick,
                ..
            )?;

            flipped_upper = self.ticks.update(
@>>             upper,
                cur_tick,
                ..
            )?;

            ...
        }
        ...

        // calculate liquidity change and the amount of each token we need
        if delta != 0 {
            let (amount_0, amount_1) = 
@>>             if self.cur_tick.get().sys() < lower { //@audit CASE - I

                // we're below the range, we need to move right, we'll need more token0
                (
                    sqrt_price_math::get_amount_0_delta(
                        tick_math::get_sqrt_ratio_at_tick(lower)?,
                        tick_math::get_sqrt_ratio_at_tick(upper)?,
                        delta,
                    )?,
                    I256::zero(),
                )
@>>         } else if self.cur_tick.get().sys() < upper { //@audit CASE - II

                // we're inside the range, the liquidity is active and we need both tokens
                let new_liquidity = liquidity_math::add_delta(self.liquidity.get().sys(), delta)?;

                self.liquidity.set(U128::lib(&new_liquidity));

                (
                    sqrt_price_math::get_amount_0_delta(
                        self.sqrt_price.get(),
                        tick_math::get_sqrt_ratio_at_tick(upper)?,
                        delta,
                    )?,
                    sqrt_price_math::get_amount_1_delta(
                        tick_math::get_sqrt_ratio_at_tick(lower)?,
                        self.sqrt_price.get(),
                        delta,
                    )?,
                )
@>>         } else { //@audit CASE - III

                // we're above the range, we need to move left, we'll need token1
                (
                    I256::zero(),
                    sqrt_price_math::get_amount_1_delta(
                        tick_math::get_sqrt_ratio_at_tick(lower)?,
                        tick_math::get_sqrt_ratio_at_tick(upper)?,
                        delta,
                    )?,
                )
            };

            Ok((amount_0, amount_1))
        } else {
            Ok((I256::zero(), I256::zero()))
        }
    }

Scene - 2 (Satisfies Case - 1 from the above snippet) Lower Tick > Upper Tick (Lower = 150, Upper = 50) // A0 => 119540 // A1 => 0

## Tools Used
Manual Review

## Recommended Mitigation Steps
```diff
    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);
        self.positions.new(id, low, up);
        Ok(())
    }

Assessed type

Invalid Validation