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

0 stars 0 forks source link

swap2ExactIn41203F1D() function won't work #235

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/lib.rs#L241

Vulnerability details

Overview

The protocol provides a facility for users to execute a 2-stage swap:

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/sol/SeawaterAMM.sol#L244

    function swap2ExactIn41203F1D(address /* tokenA */, address /* tokenB */, uint256 /* amountIn */, uint256 /* minAmountOut */) external returns (uint256, uint256) {
        directDelegate(_getExecutorSwap());
    }

This function delegate calls the following function in lib.rs:

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

    #[allow(non_snake_case)]
    pub fn swap_2_exact_in_41203_F1_D(
        &mut self,
        from: Address,
        to: Address,
        amount: U256,
        min_out: U256,
    ) -> Result<(U256, U256), Revert> {
        Pools::swap_2_internal_erc20(self, from, to, amount, min_out, None)
    }
}

During execution the following internal function is called in lib.rs:

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

    fn swap_2_internal(
        pools: &mut Pools,
        from: Address,
        to: Address,
        amount: U256,
        min_out: U256,
    ) -> Result<(U256, U256, U256, I256, i32, i32), Revert> {

    .
    .
    .

    }

Everything goes fine until LOC-241:

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

assert_eq_or!(interim_usdc_out, interim_usdc_in, Error::InterimSwapNotEq);

This assert statement would not always hold and therefore, the transaction would revert.

Proof of Concept

The following is a detail step-by-step PoC.

The issue at LOC-241 is:

assert_eq_or!(interim_usdc_out, interim_usdc_in, Error::InterimSwapNotEq);

Basically, the equality is forced to have between amountSpecified and final amount1 of a swap operation:

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

        let (amount_out, interim_usdc_in, final_tick_out) = pools.pools.setter(to).swap(
            false,
            interim_usdc_out,
            tick_math::MAX_SQRT_RATIO - U256::one(),
        )?;

This would not always hold as in the swap operation:

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

        let token0_is_input = (zero_for_one && exact_in) || (!zero_for_one && !exact_in);
        let (amount_0, amount_1) = match token0_is_input {
            true => (amount - state.amount_remaining, state.amount_calculated),
            false => (state.amount_calculated, amount - state.amount_remaining),
        };

As can be seen in the false arm:

amount1 = amountSpecified - state.amount_remaining

Therefore, amount1 != amountSpecified if state.amount_remaining != 0

So, the 2-stage swap would revert.

Severity

This issue causes a loss of functionality from users preventing them from 2-stage swaps. Thus, this is of Medium severity.

Tools Used

Manual review

Recommended Mitigation Steps

Remove the following:

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

    fn swap_2_internal(
        pools: &mut Pools,
        from: Address,
        to: Address,
        amount: U256,
        min_out: U256,
    ) -> Result<(U256, U256, U256, I256, i32, i32), Revert> {
        let original_amount = amount;

        let amount = I256::try_from(amount).map_err(|_| Error::SwapResultTooHigh)?;

        // swap in -> usdc
        let (amount_in, interim_usdc_out, final_tick_in) = pools.pools.setter(from).swap(
            true,
            amount,
            // swap with no price limit, since we use min_out instead
            tick_math::MIN_SQRT_RATIO + U256::one(),
        )?;

        // make this positive for exact in
        let interim_usdc_out = interim_usdc_out
            .checked_neg()
            .ok_or(Error::InterimSwapPositive)?;

        // swap usdc -> out
        let (amount_out, interim_usdc_in, final_tick_out) = pools.pools.setter(to).swap(
            false,
            interim_usdc_out,
            tick_math::MAX_SQRT_RATIO - U256::one(),
        )?;

        let amount_in = amount_in.abs_pos()?;
        let amount_out = amount_out.abs_neg()?;

        #[cfg(feature = "testing-dbg")]
        dbg!((
            "inside swap_2_internal",
            interim_usdc_out,
            interim_usdc_in,
            amount_out.to_string(),
            min_out.to_string()
        ));

-       assert_eq_or!(interim_usdc_out, interim_usdc_in, Error::InterimSwapNotEq);
        assert_or!(amount_out >= min_out, Error::MinOutNotReached);
        Ok((
            original_amount,
            amount_in,
            amount_out,
            interim_usdc_out,
            final_tick_in,
            final_tick_out,
        ))
    }

Assessed type

Math