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

0 stars 0 forks source link

Price is not correctly updated in the swap() function #195

Closed c4-bot-5 closed 1 month ago

c4-bot-5 commented 1 month ago

Lines of code

https://github.com/Uniswap/v3-core/blob/main/contracts/UniswapV3Pool.sol#L751

Vulnerability details

Impact

In the current implementation of the swap() function the global price is updated after calling compute_swap_step() deviating from the Uniswap implementation (as swap() function is similar to that) where price is only updated when the new tick != current tick. This can lead to a situation where price can change but the tick to stay the same which is an unexpected behavior.

Proof of Concept

Take a look at how price is updated in the protocol:

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

  self.sqrt_price.set(state.price);
        if state.tick != self.cur_tick.get().sys() {
            self.cur_tick.set(I32::unchecked_from(state.tick));
        }

However, this can create a situation where price was updated but the tick was not (also unexpected):

https://github.com/Uniswap/v3-core/blob/main/contracts/UniswapV3Pool.sol#L753-752

        if (state.tick != slot0Start.tick) {
            (uint16 observationIndex, uint16 observationCardinality) =
                observations.write(
                    slot0Start.observationIndex,
                    cache.blockTimestamp,
                    slot0Start.tick,
                    cache.liquidityStart,
                    slot0Start.observationCardinality,
                    slot0Start.observationCardinalityNext
                );
            (slot0.sqrtPriceX96, slot0.tick, slot0.observationIndex, slot0.observationCardinality) = (
                state.sqrtPriceX96,
                state.tick,
                observationIndex,
                observationCardinality
            );
        } else {
            // otherwise just update the price
            slot0.sqrtPriceX96 = state.sqrtPriceX96;
        }

The code above is the implementation that's done by UniswapV3 Pool contract where first we check ticks and if they are not equal, we don't update the price, otherwise we just push the price to the slot0 (in our case, just global sqrt_price as there is no slot0).

Tools Used

Manual review.

Recommended Mitigation Steps

Implement the price update correctly.

Assessed type

Other