V3SwapRouter.V3SwapExactOutput() has the wrong implementation since it applies the

[howlbot-integration]

Lines of code

V3SwapRouter.sol#L191-L202

Vulnerability details

Proof of Concept

First of all, let's understand how a multi-hop exact output swap should work.

In a multi-hop exact output swap, we specify the exact amount of tokens we want to receive as the final output in the last pool, and the swap logic works backward from the last pool to calculate the input needed at each preceding pool to meet this exact output requirement.

The following specifies amountOut as the exact amount for the output token for the LAST pool:

function v3SwapExactOutput(
    address recipient,
    uint256 amountOut,
    uint256 amountInMaximum,
    bytes calldata path,
    address payer
  ) internal {
    maxAmountInCached = amountInMaximum;
    (int256 amount0Delta, int256 amount1Delta, bool zeroForOne) =
      _swap(-amountOut.toInt256(), recipient, path, payer, false);        // first argument is the exact output amount as ```-amountOut```

    uint256 amountOutReceived = zeroForOne ? uint256(-amount1Delta) : uint256(-amount0Delta);

    if (amountOutReceived != amountOut) revert V3InvalidAmountOut();

    maxAmountInCached = DEFAULT_MAX_AMOUNT_IN;
  }

It calls _swap() using a negative integer -amountOut as the first argument.

The problem lies in the implementation in _swap():

function _swap(int256 amount, address recipient, bytes calldata path, address payer, bool isExactIn)
    private
    returns (int256 amount0Delta, int256 amount1Delta, bool zeroForOne)
  {
    (address tokenIn, uint24 fee, address tokenOut) = path.decodeFirstPool();

    zeroForOne = isExactIn ? tokenIn < tokenOut : tokenOut < tokenIn;

    (amount0Delta, amount1Delta) = IKatanaV3Pool(computePoolAddress(tokenIn, tokenOut, fee)).swap( // slippage control is relaxed
      recipient, zeroForOne, amount, (zeroForOne ? MIN_SQRT_RATIO + 1 : MAX_SQRT_RATIO - 1), abi.encode(path, payer)
    );
  }

Instead of passing the first argument amount, which is supposed to be the exact amount for the output token for the LAST pool, it applies it as the argument for calling the swap function of the FIRST pool. As a result, This results in the first pool using amountOut as the required output, causing incorrect calculations across the multi-hop path.

Impact: loss of funds due to wrong calculation or non-function of the transaction.

Recommended Mitigation Steps

Start with the Last Pool: Decode and apply amountOut to the last pool in the path, calculating backward to determine the required input for each previous pool.

Assessed type

Math

[khangvv]

When executing the V3_SWAP_EXACT_OUT command, the path is expected to be in reverse order, as noted in this reference. For context, we’ve maintained Uniswap’s original implementation for this part of the code without modifications, ensuring compatibility with any Uniswap SDK for constructing swap parameters.

[khanhtaskymaviscom]

As @khangvv mentioned, path.decodeFirstPool() actually returns "the LAST pool" in your term, so it should be correct here.

[alex-ppg]

The Warden attempts to outline a vulnerability in the way multi-hop exact output swaps are executed which stems from a misunderstanding of the original Uniswap V3 system this behavior has been copied from. The path is specified in an inverse direction to facilitate its gas-efficient processing as correctly implemented in the current code.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid