Incorrect Slippage Check in swapInPermit2CEAAB576 Function Allows Excessive Slippage, Risking User Funds

[c4-bot-8]

Lines of code

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

Vulnerability details

Summary

The SeawaterAMM contract implements a swapInPermit2CEAAB576 function for token swaps using the Permit2 standard. This function is designed to perform a swap with slippage protection.

The slippage check in the swapInPermit2CEAAB576 function is incorrect. It compares the negated output amount with the minimum expected output, which doesn't accurately represent the slippage protection intended.

Code Snippet

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

function swapInPermit2CEAAB576(address token, uint256 amountIn, uint256 minOut, uint256 nonce, uint256 deadline, uint256 maxAmount, bytes memory sig) external returns (int256, int256) {
    // ... (delegatecall and decoding logic)

    (int256 swapAmountIn, int256 swapAmountOut) = abi.decode(data, (int256, int256));
    // this contract uses checked arithmetic, this negate can revert
    require(-swapAmountOut >= int256(minOut), "min out not reached!");
    return (swapAmountIn, swapAmountOut);
}

Impact

This bug could lead to trades executing with more slippage than the user intended, potentially resulting in significant financial losses for users. In extreme market conditions, this could allow trades to go through even when they shouldn't, violating the user's specified slippage tolerance.

Scenario

1. A user wants to swap 100 TokenA for at least 90 TokenB.
2. They call swapInPermit2CEAAB576 with amountIn = 100 and minOut = 90.
3. Due to market conditions, the actual output is 85 TokenB.
4. The function receives swapAmountOut = -85 (negative to represent outgoing tokens).
5. The check -swapAmountOut >= int256(minOut) becomes 85 >= 90.
6. This condition is false, but due to the incorrect comparison, the trade still goes through.
7. The user receives fewer tokens than their specified minimum, violating their slippage tolerance.

Fix

The slippage check should compare the absolute value of swapAmountOut with minOut.

require(uint256(-swapAmountOut) >= minOut, "min out not reached!");

This ensures that the actual output amount is compared correctly with the user's specified minimum.

Assessed type

Invalid Validation

[blckhv]

Hey @alex-ppg,

I would like to escalate this issue on behalf of the submitter. This is a duplicate of https://github.com/code-423n4/2024-08-superposition-findings/issues/35.

Thanks!

[alex-ppg]

Hey @blckhv, thank you for your PJQA contribution. I would like to clarify that validation repository findings are not directly evaluated by the judge of a contest if they do not pass the validation phase.

It appears that this particular finding relates to the swap-in function that has been correctly implemented.