Maverick contest - rvierdiiev's results

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/router-v1/contracts/Router.sol#L277-L287

Vulnerability details

Impact

Router.getOrCreatePoolAndAddLiquidity can be frontrunned which allows to set different initial price.

Proof of Concept

When new Pool is created it is already initialized with active tick. This allows the pool creator to provide price for assets. So when first LP calls Pool.addLiquidity, this active tick [is used]( to determine how many assets LP should deposit.

Router.getOrCreatePoolAndAddLiquidity function allows caller to add liquidity to the pool that can be not created yet. In such case it will create it with initial active tick provided by sender and then it will provide user's liquidity to the pool. In case if pool already exists, function will just add liquidity to it.

https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/router-v1/contracts/Router.sol#L277-L287

    function getOrCreatePoolAndAddLiquidity(
        PoolParams calldata poolParams,
        uint256 tokenId,
        IPool.AddLiquidityParams[] calldata addParams,
        uint256 minTokenAAmount,
        uint256 minTokenBAmount,
        uint256 deadline
    ) external payable checkDeadline(deadline) returns (uint256 receivingTokenId, uint256 tokenAAmount, uint256 tokenBAmount, IPool.BinDelta[] memory binDeltas) {
        IPool pool = getOrCreatePool(poolParams);
        return addLiquidity(pool, tokenId, addParams, minTokenAAmount, minTokenBAmount);
    }

1.Pool of tokenA:tokenB doens't exist. 2.User calls Router.getOrCreatePoolAndAddLiquidity function to create pool with initial active tick and provide liquidity in same tx. 3.Attacker frontruns tx and creates such pool with different active tick. 4.Router.getOrCreatePoolAndAddLiquidity is executed and new Pool was not created, and liquidity was added to the Pool created by attacker. 5.Attacker swapped provided by first depositor tokens for a good price. 6.First depositor lost part of funds.

Tools Used

VsCode

Recommended Mitigation Steps

Do not allow to do such actions together in one tx. Do it in 2 tx. First, create Pool. And in second tx add liquidity.

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/router-v1/contracts/Router.sol#L132-L140 https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/router-v1/contracts/Router.sol#L122

Vulnerability details

Impact

If user calls Router.exactInputSingle swap with params.recipient == 0 by mistake, all swapping funds will be sent to Router and anyone can claim them to himself.

Proof of Concept

Router.exactInputSingle function calls exactInputInternal function and pases recipient param. https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/router-v1/contracts/Router.sol#L121-L129

    function exactInputInternal(uint256 amountIn, address recipient, uint256 sqrtPriceLimitD18, SwapCallbackData memory data) private returns (uint256 amountOut) {
        if (recipient == address(0)) recipient = address(this);


        (IERC20 tokenIn, IERC20 tokenOut, IPool pool) = data.path.decodeFirstPool();


        bool tokenAIn = tokenIn < tokenOut;


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

As you can see if recipient is 0, then it is set to address(this). That means that receiver of trade will be Router. Router contract has several methods that allow anyone to transfer controlled by Router tokens to himself, like refundETH, sweepToken, unwrapWETH9. https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/router-v1/contracts/Router.sol#L70-L77

    function sweepToken(IERC20 token, uint256 amountMinimum, address recipient) public payable {
        uint256 balanceToken = token.balanceOf(address(this));
        require(balanceToken >= amountMinimum, "Insufficient token");


        if (balanceToken > 0) {
            TransferHelper.safeTransfer(address(token), recipient, balanceToken);
        }
    }

No restrictions here, so anyone can call.

This creates possiblity that after user will make exactInputSingle swap with recipient 0 address and in another tx will try to sweep those tokens, someone will frontrun user and will send tokens to himself. User will lost swapped funds.

This test will show that if recipient is 0 then after the swap Router controls swapped tokens. You can run it inside Router.ts.

it.only("swap recipient 0", async () => {
      let _fee = 0.01 / 100;
      let pool: string = await getPool(_fee, tokenA, tokenB, 500, 500);
      const prePoolBalance = await balances(pool);
      const preRouterBalance = await balances(router.address);
      //Router balance of b token is 0
      expect(preRouterBalance.tokenB).to.be.eq(0);
      console.log(preRouterBalance);
      expect(
        await router.exactInputSingle([
          tokenA.address,
          tokenB.address,
          pool,
          ethers.constants.AddressZero,
          maxDeadline,
          floatToFixed(10),
          0,
          0,
        ])
      )
        .to.emit(tokenA, "Transfer")
        .to.emit(tokenB, "Transfer");
      const postPoolBalance = await balances(pool);
      expect(postPoolBalance.tokenA - prePoolBalance.tokenA).to.eq(10);
      expect(postPoolBalance.tokenB - prePoolBalance.tokenB).to.be.lessThan(-4);

      const postRouterBalance = await balances(router.address);
      console.log(postRouterBalance);
      //Router balance of b token is not 0 anymore
      //now anyone can sweep it
      expect(postRouterBalance.tokenB).to.be.not.eq(0);
    });

Tools Used

VsCode

Recommended Mitigation Steps

Add check to exactInputSingle and exactOutputSingle that params.recipient is not 0.