Lines of code

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

Vulnerability details

Impact

A possible wrong amount of the amount already swapped in/out of the input/output asset of a swap due to an unintended under/overflow.

Proof of Concept

In Rust release mode, the - and + operation will not revert when an underflow/overflow occurs.

As such, the amount can under/overflow in the while loop:

    match exact_in {
        true => { 
            state.amount_remaining -=
                I256::unchecked_from(step_amount_in + step_fee_amount);
->          state.amount_calculated -= I256::unchecked_from(step_amount_out); //@audit this can underflow with -=
        }
        false => {
            state.amount_remaining += I256::unchecked_from(step_amount_out);
->          state.amount_calculated += //@audit this can overflow with +=
                I256::unchecked_from(step_amount_in + step_fee_amount); 
        }
    }

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

The issue is that these are not meant to under/overflow as seen in the original Uniswap logic, as they use SafeMath:

    if (exactInput) {
        state.amountSpecifiedRemaining -= (step.amountIn + step.feeAmount).toInt256();
->      state.amountCalculated = state.amountCalculated.sub(step.amountOut.toInt256());
    } else {
        state.amountSpecifiedRemaining += step.amountOut.toInt256();
->      state.amountCalculated = state.amountCalculated.add((step.amountIn + step.feeAmount).toInt256());
    }

https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/UniswapV3Pool.sol#L673-L679

Uniswap tests that state.amountSpecifiedRemaining > step.amountIn + step.feeAmount and that's why the operation is safe to use unchecked, but this is not the case with the second one, which must be checked:

https://github.com/Uniswap/v3-core/blob/0.8/contracts/UniswapV3Pool.sol#L681-L685

Tools Used

Manual Review

Recommended Mitigation Steps

Consider using checked_sub instead of - (and checked_add instead of +):

    match exact_in {
        true => { 
            state.amount_remaining -=
                I256::unchecked_from(step_amount_in + step_fee_amount);
-           state.amount_calculated -= I256::unchecked_from(step_amount_out);
+           state.amount_calculated = state.amount_calculated.checked_sub(I256::unchecked_from(step_amount_out));
        }
        false => {
            state.amount_remaining += I256::unchecked_from(step_amount_out);
-           state.amount_calculated +=
-               I256::unchecked_from(step_amount_in + step_fee_amount);
+           state.amount_calculated = state.amount_calculated.checked_add(
+               I256::unchecked_from(step_amount_in + step_fee_amount));
        }
    }

Assessed type

Uniswap

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

Unintended under/overflow of the amount already swapped in/out due to unmatching logic #185