UniSwapper.swapExactOut allows Sandwich attacks (front-run and back-run)

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L143

Vulnerability details

Impact

Loss of funds.

Proof of Concept

UniSwapper.swapExactOut

The function swapExactOut tries to do the swap in the same block that it is initiated considering that the deadline is block.timestamp, as shown at [1], this approach carries a lot of Sandwich attacks (front-run and back-run).

This is mainly possible because the lack of a timelock to protect against front-runs and also an expiration time to protect against back-runs. Important to note that the system uses UniSwap V2 and the timelock is a mechanism used by UniSwap V2.

function swapExactOut(
    SwapParams memory swapParams,
    address receiver,
    address refundAddress
  ) public payable routerIsSet returns (uint256 amountIn) {
    swapParams = _receiveAndWrapIfNeeded(swapParams);
    IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter
      .ExactOutputParams({
        path: swapParams.path,
        recipient: address(this),
        //deadline: block.timestamp,            //[1]
        amountOut: swapParams.amountOut,
        amountInMaximum: swapParams.amountIn
      });

    IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);
    amountIn = IV3SwapRouter(uniswap_router).exactOutput(params);

    // refund sender
    _refundUser(
      refundAddress,
      swapParams.tokenIn,
      params.amountInMaximum - amountIn
    );

    _sendToRecipient(receiver, swapParams.tokenOut, swapParams.amountOut);
  }

Tools Used

Manual review

Recommended Mitigation Steps

Patch

• This code will add a timelock of 1 minute to the swap and an expiration time of 2 minutes. This means that the swap will not be executed until at least one minute after it is initiated. If the swap is not executed within two minutes, it will be reverted. This will help to protect against back-runs.

function swapExactOut(
  SwapParams memory swapParams,
  address receiver,
  address refundAddress
) public payable routerIsSet returns (uint256 amountIn) {
  swapParams = _receiveAndWrapIfNeeded(swapParams);
  IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter
    .ExactOutputParams({
      path: swapParams.path,
      recipient: address(this),
      deadline: block.timestamp + 1 minutes, // Add a timelock of 1 minute
      expirationTime: block.timestamp + 2 minutes // Set an expiration time of 2 minutes
    });

  IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);
  amountIn = IV3SwapRouter(uniswap_router).exactOutput(params);

  // refund sender
  _refundUser(
    refundAddress,
    swapParams.tokenIn,
    params.amountInMaximum - amountIn
  );

  // send to recipient
  _sendToRecipient(receiver, swapParams.tokenOut, swapParams.amountOut);
}

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #37

[raymondfam]

Inadequate elaboration. See also #24 on deadline preference.

[c4-judge]

alex-ppg marked the issue as not a duplicate

[alex-ppg]

The Warden attempts to describe a potential attack on the swaps performed by the UniSwapper, however, the code they include is invalid and the trades are generally regarded as safe given that they evaluate their minimum outputs / maximum inputs. As such, I consider this submission invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid