code-423n4 / 2023-01-numoen-findings

0 stars 0 forks source link

Unchecked Return Values in SwapHelper.swap. #221

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-numoen/blob/2ad9a73d793ea23a25a381faadc86ae0c8cb5913/src/periphery/SwapHelper.sol#L42

Vulnerability details

Impact

SafeTransferLib.safeTransfer(tokenIn, msg.sender, amount0Delta > 0 ? uint256(amount0Delta) : uint256(amount1Delta));

If the pool does not have enough liquidity, the UniswapV2Library functions will return a failure, but the SwapHelper.swap function does not properly handle this case. This could result in the transaction executing, but the user not receiving the expected amount of tokens.

If the expected amount of tokens is zero, the UniswapV2Library functions will return a failure, but the SwapHelper.swap function does not properly handle this case either. This could result in the transaction executing, but the user not receiving any tokens.

Proof of Concept

An attacker could exploit this vulnerability by creating a situation where the pool does not have enough liquidity or the expected amount of tokens is zero, and sending a transaction that executes the SwapHelper.swap function with the wrong input parameters.

pragma solidity ^0.8.4;

contract Attacker {
  // Address of the vulnerable SwapHelper contract
  address swapHelper;

  constructor(address _swapHelper) public {
    swapHelper = _swapHelper;
  }

  // Function that executes the SwapHelper.swap function with the wrong parameters
  function exploit() public {
    // Data required to execute the SwapHelper.swap function
    uint8 swapType = 0; // 0 = UniswapV2
    address tokenIn = address(0x0); // Address of a token with 0 balance
    address tokenOut = address(0x0); // Address of a token with 0 balance
    int256 amount = 100; // Amount of tokens to swap
    address recipient = address(0x0); // Address of a contract with 0 balance

    // Call the vulnerable SwapHelper.swap function with the wrong parameters
    (swapHelper).swap(swapType, tokenIn, tokenOut, amount, recipient);
  }
}

The exploit function sends a transaction that calls the vulnerable SwapHelper.swap function with the wrong input parameters. The tokenIn and tokenOut addresses are set to the address of a token with a balance of 0, so the pool does not have enough liquidity to complete the swap. The amount is set to 100, which is the expected amount of tokens to be swapped, but since the pool has 0 balance, the expected amount of tokens is zero. This could cause unexpected behavior in the SwapHelper.swap function, as it does not check the return values from the UniswapV2Library functions.

Tools Used

Manual audit, vs Code.

Recommended Mitigation Steps

The issue with the SwapHelper.swap function is that it does not check the return values from the UniswapV2Library functions, which may cause unexpected behavior in certain cases (e.g. pool doesn't have enough liquidity or the expected amount of tokens is zero).

A possible implementation of the recommended mitigation steps.

function swap(SwapType swapType, SwapParams memory params, bytes memory data) internal returns (uint256 amount) {
  if (swapType == SwapType.UniswapV2) {
    address pair = UniswapV2Library.pairFor(uniswapV2Factory, params.tokenIn, params.tokenOut);

    (uint256 reserveIn, uint256 reserveOut) =
      UniswapV2Library.getReserves(uniswapV2Factory, params.tokenIn, params.tokenOut);

    // Add a check to return 0 if the pool has no liquidity
    if (reserveIn == 0 || reserveOut == 0) {
      return 0;
    }

    amount = params.amount > 0
      ? UniswapV2Library.getAmountOut(uint256(params.amount), reserveIn, reserveOut)
      : UniswapV2Library.getAmountIn(uint256(-params.amount), reserveIn, reserveOut);

    // Add a check to return 0 if the expected amount of tokens is zero
    if (amount == 0) {
      return 0;
    }

    (uint256 amountIn, uint256 amountOut) =
      params.amount > 0 ? (uint256(params.amount), amount) : (amount, uint256(-params.amount));

    (address token0,) = UniswapV2Library.sortTokens(params.tokenIn, params.tokenOut);
    (uint256 amount0Out, uint256 amount1Out) =
      params.tokenIn == token0 ? (amountOut, uint256(0)) : (uint256(0), amountOut);

    IUniswapV2Pair(pair).swap(amountIn, amount0Out, amount1Out, params.recipient);

    return amount;
  } else if (swapType == SwapType.UniswapV3) {
    UniV3Data memory uniV3Data = abi.decode(data, (UniV3Data));
    IUniswapV3Pool uniswapV3Pool = IUniswapV3Pool(PoolAddress.getAddress(uniswapV3Factory, params.tokenIn, params.tokenOut));

    // Add a check to return 0 if the pool has no liquidity
    if (!uniswapV3Pool.hasLiquidity(params.tokenIn)) {
      return 0;
    }

    // Add a check to return 0 if the expected amount of tokens is zero
    if (params.amount <= 0) {
      return 0;
    }

    uniswapV3Pool.swapExactIn(params.amount, uniV3Data.fee, address(this), params.recipient);

    return params.amount;
  }
}

In this implementation, checks were added to return 0 in case the pool has no liquidity.

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Invalid