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

0 stars 0 forks source link

Invariant Broken: Pools can be created with `upper tick <= lower tick` #190

Closed c4-bot-1 closed 1 month ago

c4-bot-1 commented 1 month ago

Lines of code

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

Vulnerability details

Impact

The function create_position() is used to create a new position in the pool. The function takes lower tick and upper tick as input params to create a new position in the pool. The checks which verifies the ticks aren't invalid are:

        assert_or!(low >= min_tick && low <= max_tick, Error::InvalidTick);
        assert_or!(up >= min_tick && up <= max_tick, Error::InvalidTick);

But the issue is this implementation is differs from the Uniswap V3 implementation and allows ticks to be invalid as function accepts low > up

Uniswap V3 implementation:

    /// @dev Common checks for valid tick inputs.
    function checkTicks(int24 tickLower, int24 tickUpper) private pure {
        require(tickLower < tickUpper, 'TLU');
        require(tickLower >= TickMath.MIN_TICK, 'TLM');
        require(tickUpper <= TickMath.MAX_TICK, 'TUM');
    }

The contest's README states the main invariants of the protocols as:

Only valid tick ranges, and fee structures, are accessible.

This vulnerability allows the function caller to put invalid tick ranges by providing a upper tick < lower tick.

Proof of concept

If we look at the function create_position

    pub fn create_position(&mut self, id: U256, low: i32, up: i32) -> Result<(), Revert> {
        ...
        ...

        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 check is incorrect. should be low <= up
        // and up >= low
        // or add : low <= up

        self.positions.new(id, low, up);
        Ok(())
    }

It clearly lacks the validation of up > low similar to Uniswap V3 implementation.

Tools Used

Manual Review

Recommendation

We recommend to add the checks to verify that the up > low similar to Uniswap V3 core contract implementation like this: https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/UniswapV3Pool.sol#L126

Assessed type

Invalid Validation