`SemiFungiblePositionManager.swapInAMM` FUNCTION DOES NOT IMPLEMENT EITHER SLIPPAGE PROTECTION OR DEADLINE PROTECTION THUS CREATING LOSS OF FUNDS TO THE `msg.sender`

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L824-L832 https://github.com/code-423n4/2023-11-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L706-L708

Vulnerability details

Impact

The SemiFungiblePositionManager._validateAndForwardToAMM function is used to create the positions in the AMM by calling the _createPositionInAMM function. The return values of the function call provides the itmAmounts which denotes the In-The-Money token amounts (amount0 and amount1) for the given position.

When the (itmAmounts != 0) && (swapAtMint) condition is true the SemiFungiblePositionManager.swapInAMM function is called to swap the itmAmounts given in token1 to token0 amount or vice versa, by calling the _univ3swap.swap function as shown below:

        (int256 swap0, int256 swap1) = _univ3pool.swap( //@audit-info - swap using the univ3pool
            msg.sender,
            zeroForOne, //@audit-info - If true, swap direction is from token0 -> token1 
            swapAmount, //@audit-info - the sign decides whether it is an exact Input or exact Output. - -> exactOutput, + -> exactInput
            zeroForOne
                ? Constants.MIN_V3POOL_SQRT_RATIO + 1 //@audit-info - Minimum possible sqrtPriceX96 in a Uniswap V3 pool
                : Constants.MAX_V3POOL_SQRT_RATIO - 1, //@audit-info - Maximum possible sqrtPriceX96 in a Uniswap V3 pool
            data
        );

The issue with this implementation is there is no slippage protection or deadline protection for the swapped itmAmount value in the swapInAMM function.

Due to lack of slippage check the received totalSwapped amount will be very less for the output token due to dynamic nature of transaction happening in the univ3pools. Hence this could be loss of funds to the msg.sender.

Furthermore due to the lack of deadline check the following two scenarios could occur:

1. For example if the gasfee provided to the exchange transaction is low for the miners due to high average gas fee present in the mempool due to congestion. Hence the exchange transaction will only execute once the average gas fees drop so the miners will be interested in executing this transaction. But by then the underlying value of the output token would have dropped due to changing market dynamics (For example if the output token is weth then the underlying value of weth denominated in USD might have dropped by 200USD by the time exchange transaction was executed after the delay).

2. The MEV bots can perform sandwich attacks on the exchange trade. If the weth price has dropped compared to the input token significantly thus allowing the trader to gain more weth tokens after the exchange. But since the exchange transaction is performed after a delay the slippage protection gets outdated which would allow significant slippage. As a result a MEV bot could exploit this sandwiching the exchange transaction resulting in significant profit and large loss to the trader due to outdated slippage protection.

Proof of Concept

            (int256 swap0, int256 swap1) = _univ3pool.swap(
                msg.sender,
                zeroForOne,
                swapAmount,
                zeroForOne
                    ? Constants.MIN_V3POOL_SQRT_RATIO + 1
                    : Constants.MAX_V3POOL_SQRT_RATIO - 1,
                data
            );

https://github.com/code-423n4/2023-11-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L824-L832

        if ((itmAmounts != 0) && (swapAtMint)) {
            totalMoved = swapInAMM(univ3pool, itmAmounts).add(totalMoved);
        }

https://github.com/code-423n4/2023-11-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L706-L708

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence while interacting with DEXes like uniswapv3 it is recommended to implement both the slippage and deadline protection.

It is recommended to implement the slippage protection separately in the SemiFungiblePositionManager.swapInAMM function (since univ3pool.swap function do not inherently have slippage protection implemented). This can be done by passing in a minimum output amount (as input parameter to the swapInAMM function) expected after the swap and revert the transaction if the actual returned swap amount is less than this minimum output amount.

It is recommended to implement the deadline protection separately in the SemiFungiblePositionManager.swapInAMM function (since univ3pool.swap function do not inherently have deadline protection implemented). This can be done by passing in a input timestamp by the user indicating the latest time the swap transaction can be executed and if the current block.timestamp has exceeded this timestamp then the transaction should revert.

Assessed type

Other

[c4-judge]

Picodes marked the issue as primary issue

[c4-judge]

Picodes marked issue #304 as primary and marked this issue as a duplicate of 304

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid