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

0 stars 0 forks source link

Ticks are allowed to be over `int24` deviating form intended uniswap standard #252

Closed c4-bot-10 closed 1 month ago

c4-bot-10 commented 1 month ago

Lines of code

https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/README.md#L141-L148

Vulnerability details

Proof of Concept

A main invariant in scope is to have Uniswap maths followed faithfully: https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/README.md#L141-L148

## Main invariants

- We should follow Uniswap V3's math faithfully.

Now protocol itself includes multiple logic that deals with ticks, bitmaps and all, now per Uniswap's faithful maths to be followed, we need the size of these values, be it ticks, and their spacing to be the same.

This is because Uniswap's tickmath allows for overflowing logic and in this case when any calculation regarding the tick is at int24.max/min it overflows.

For example see https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/libraries/TickBitmap.sol#L42-L77

Superposition's current implementation however does not ensure this, which is because it assumes an int32 value for it's ticks, see https://github.com/code-423n4/2024-08-superposition/blob/4528c9d2dbe1550d2660dac903a8246076044905/pkg/seawater/src/maths/tick_bitmap.rs#L14-L69

pub fn next_initialized_tick_within_one_word(
    tick_bitmap: &TickBitmap,
    tick: i32,
    tick_spacing: i32,
    lte: bool,
) -> Result<(i32, bool), Error> {
    let compressed = if tick < 0 && tick % tick_spacing != 0 {
        (tick / tick_spacing) - 1
    } else {
        tick / tick_spacing
    };

    if lte {
        let (word_pos, bit_pos) = position(compressed);

        let mask = (U256::one() << bit_pos) - U256::one() + (U256::one() << bit_pos);

        let masked = tick_bitmap.get(word_pos) & mask;

        let initialized = !masked.is_zero();

        let next = if initialized {
            (compressed
                - (bit_pos
                    .overflowing_sub(bit_math::most_significant_bit(masked)?)
                    .0) as i32)
                * tick_spacing
        } else {
            (compressed - bit_pos as i32) * tick_spacing
        };

        Ok((next, initialized))
    } else {
        let (word_pos, bit_pos) = position(compressed + 1);

        let mask = !((U256::one() << bit_pos) - U256::one());

        let masked = tick_bitmap.get(word_pos) & mask;

        let initialized = !masked.is_zero();

        let next = if initialized {
            (compressed
                + 1
                + (bit_math::least_significant_bit(masked)?
                    .overflowing_sub(bit_pos)
                    .0) as i32)
                * tick_spacing
        } else {
            (compressed + 1 + ((0xFF - bit_pos) as i32)) * tick_spacing
        };

        Ok((next, initialized))
    }
}

The above easily allows a scenario where we can see how the main invariant is broken:

Impact

Core invariant is broken.

Recommended Mitigation Steps

To maintain religiously following Uniswap's math then there is a need to also use the specified int/uint sizes for variables.

Assessed type

Context