search

code-423n4 / 2023-11-zetachain-findings

0 stars 0 forks source link

Some tokens can be stuck in the consumer contract or router (in case of ETH), due to low liquidity in the Uniswap V3 pool #57

Closed c4-submissions closed 10 months ago

c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol#L74-L183 https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L103-L207

Vulnerability details

Impact

During swap operations the user needs to transfer tokens to the consumer contract or send ETH directly. https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol#L107-L118

        IERC20(inputToken).safeTransferFrom(msg.sender, address(this), inputTokenAmount);
        IERC20(inputToken).safeApprove(address(uniswapV3Router), inputTokenAmount);

        ISwapRouter.ExactInputParams memory params = ISwapRouter.ExactInputParams({
            deadline: block.timestamp + MAX_DEADLINE,
            path: abi.encodePacked(inputToken, tokenPoolFee, WETH9Address, zetaPoolFee, zetaToken),
            recipient: destinationAddress,
            amountIn: inputTokenAmount,
            amountOutMinimum: minAmountOut
        });

        uint256 amountOut = uniswapV3Router.exactInput(params);

after that, contract will attempt to swap tokens on Uniswap. sqrtPriceLimitX96 is set to zero, this way router attempts to utilize all liquidity in the pool. However, there is a possibility that there is not enough liquidity in the pool for a given amount, in which case the swap will be partially executed. https://github.com/Uniswap/v3-core/blob/main/contracts/UniswapV3Pool.sol#L640-L650

// continue swapping as long as we haven't used the entire input/output and haven't reached the price limit
while (state.amountSpecifiedRemaining != 0 && state.sqrtPriceX96 != sqrtPriceLimitX96) {
    StepComputations memory step;
    step.sqrtPriceStartX96 = state.sqrtPriceX96;
    (step.tickNext, step.initialized) = tickBitmap.nextInitializedTickWithinOneWord(
        state.tick,
        tickSpacing,
        zeroForOne
    );

What happens to tokens provided by the user?

Proof of Concept

Here is the case for the ETH->ZETA swap written for Forge. 

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.7;

import { ZetaTokenConsumerUniV3, IERC20 } from "../contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol";
import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "forge-std/console.sol";
import "forge-std/Test.sol";

interface INonfungiblePositionManager {
    struct MintParams {
        address token0;
        address token1;
        uint24 fee;
        int24 tickLower;
        int24 tickUpper;
        uint256 amount0Desired;
        uint256 amount1Desired;
        uint256 amount0Min;
        uint256 amount1Min;
        address recipient;
        uint256 deadline;
    }
    function mint(MintParams calldata params)
        external
        payable
        returns (
            uint256 tokenId,
            uint128 liquidity,
            uint256 amount0,
            uint256 amount1
    );
    function createAndInitializePoolIfNecessary(
        address token0,
        address token1,
        uint24 fee,
        uint160 sqrtPriceX96
    ) external payable returns (address pool);
}

interface IWETH is IERC20 {
    function deposit() external payable;
}

interface IV3Router {
    function factory() external returns(address);
}

contract MockToken is ERC20 {
    constructor(string memory name, string memory symbol) ERC20(name, symbol) {} 
    function mint(address to, uint256 amount) external {
        _mint(to, amount);
    }
}

contract C4 is Test{
    address alice = makeAddr("alice");
    IV3Router router;
    IWETH WETH;
    INonfungiblePositionManager positionManager;
    MockToken ZETA;
    ZetaTokenConsumerUniV3 consumer;
    address poolAddress;

    function setUp() public {
        vm.createSelectFork('https://rpc.ankr.com/eth');
        router = IV3Router(0xE592427A0AEce92De3Edee1F18E0157C05861564);
        WETH = IWETH(0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2);
        positionManager = INonfungiblePositionManager(0xC36442b4a4522E871399CD717aBDD847Ab11FE88);

        ZETA = new MockToken("ZETA", "ZETA");
        ZETA.mint(address(this), 100_000e18);
        WETH.deposit{value: 100e18}();

        ZETA.approve(address(positionManager), ~uint256(0));
        WETH.approve(address(positionManager), ~uint256(0));

        poolAddress = positionManager.createAndInitializePoolIfNecessary(
            address(ZETA),
            address(WETH),
            uint24(3000),
            uint160(2505414483750479311864138015)
        );

        INonfungiblePositionManager.MintParams memory params =
            INonfungiblePositionManager.MintParams({
                token0: address(ZETA),
                token1: address(WETH),
                fee: uint24(3000),
                tickLower: -887220,
                tickUpper: 887220,
                amount0Desired: 100,
                amount1Desired: 1,
                amount0Min: 0,
                amount1Min: 0,
                recipient: address(this),
                deadline: block.timestamp
            });

        (uint256 tokenId, , , ) = positionManager.mint(params);
        consumer = new ZetaTokenConsumerUniV3(
            address(ZETA),
            address(router),
            0x1F98431c8aD98523631AE4a59f267346ea31F984,
            address(WETH),
            uint24(3000),
            uint24(3000)
        );
    }

    function testPartialSwap() public {
        // not enough liquidity to fill whole order
        consumer.getZetaFromEth{value: 100e18}(alice, 0);
        // some amount of ETH will stay in router
        console.log("ROUTER ETH: ", address(router).balance);
    }
}

Tools Used

Foundry, forge-std lib

Recommended Mitigation Steps

We need to send the excess tokens back to msg.sender or call router function refundETH after the swap.

Assessed type

Uniswap

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as primary issue

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as insufficient quality report

c4-judge commented 10 months ago

0xean marked the issue as unsatisfactory: Insufficient quality

c4-judge commented 10 months ago

0xean marked the issue as unsatisfactory: Insufficient quality

k4zanmalay commented 10 months ago

@0xean hey! I was referring to the issue described in this post https://jeiwan.net/posts/public-bug-report-uniswap-swaprouter/. There is a possibility that there is not enough liquidity in ZETA/WETH pool to fully utilize provided token amount, as a result uniswapV3Router.exactInput(params) will be executed partially. In case of EOA, unutilized tokens are returned back, but since we are using a consumer contract, they will be sent to it's address and there is no way to retrieve them back.

Also there is a same issue in history that was judged as a valid Medium https://github.com/code-423n4/2023-05-juicebox-findings/issues/162

Thank you!

0xean commented 10 months ago

If a reasonable min amount out is supplied, the transaction simply reverts.

   consumer.getZetaFromEth{value: 100e18}(alice, 0);

you are calling it with 0 as the min amount out which is user error.