c4-bot-6 commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L110 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L25 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L151

Vulnerability details

Impact

The identified issue lies in the getZetaFromEth function, specifically when interacting with PancakeSwap. The vulnerability stems from the absence of a deadline parameter in the ExactInputSingleParams struct, exposing the trade to potential manipulation by miners and MEV (Miner Extractable Value) attacks. Without a deadline parameter, the trade execution becomes susceptible to front-running and sandwich attacks. Miners could prioritize transactions strategically, potentially leading to undesirable outcomes such as unfavorable rates and increased slippage.

Proof of Concept

interface ISwapRouter is IUniswapV3SwapCallback {
    struct ExactInputSingleParams {
        address tokenIn; -----------------
        address tokenOut;                |
        uint24 fee;                      |
        address recipient;               |
        uint256 deadline;                |<|-----@note complete function for swapping
        uint256 amountIn;                |
        uint256 amountOutMinimum;        |
        uint160 sqrtPriceLimitX96;--------
    }

Incomplete forked interface from Uniswap

interface ISwapRouterPancake is IUniswapV3SwapCallback {
    struct ExactInputSingleParams {
        address tokenIn;
        address tokenOut;
        uint24 fee;
        address recipient; <------ @audit no deadline parameter
        uint256 amountIn;
        uint256 amountOutMinimum;
        uint160 sqrtPriceLimitX96;
    }

ISwapRouterPancake.ExactInputSingleParams memory params = ISwapRouterPancake.ExactInputSingleParams({
    tokenIn: WETH9Address,
    tokenOut: zetaToken,
    fee: zetaPoolFee, // @audit no deadline paramter
    recipient: destinationAddress,
    amountIn: msg.value,
    amountOutMinimum: minAmountOut,
    sqrtPriceLimitX96: 0
});

 function getEthFromZeta(
        address destinationAddress,
        uint256 minAmountOut,
        uint256 zetaTokenAmount
    ) external override returns (uint256) {
        if (destinationAddress == address(0)) revert ZetaCommonErrors.InvalidAddress();
        if (zetaTokenAmount == 0) revert InputCantBeZero();

        IERC20(zetaToken).safeTransferFrom(msg.sender, address(this), zetaTokenAmount);
        IERC20(zetaToken).safeApprove(address(pancakeV3Router), zetaTokenAmount);

        ISwapRouterPancake.ExactInputSingleParams memory params = ISwapRouterPancake.ExactInputSingleParams({
            tokenIn: zetaToken,
            tokenOut: WETH9Address,
            fee: zetaPoolFee,
            recipient: address(this), // @audit missing here also!
            amountIn: zetaTokenAmount,
            amountOutMinimum: minAmountOut,
            sqrtPriceLimitX96: 0
        });

uint256 amountOut = pancakeV3Router.exactInputSingle{value: msg.value}(params);

 ISwapRouterPancake.ExactInputParams memory params = ISwapRouterPancake.ExactInputParams({
            path: abi.encodePacked(zetaToken, zetaPoolFee, WETH9Address, tokenPoolFee, outputToken),
            recipient: destinationAddress,
            amountIn: zetaTokenAmount,
            amountOutMinimum: minAmountOut
        });

  ISwapRouterPancake.ExactInputParams memory params = ISwapRouterPancake.ExactInputParams({
            path: abi.encodePacked(inputToken, tokenPoolFee, WETH9Address, zetaPoolFee, zetaToken),
            recipient: destinationAddress,
            amountIn: inputTokenAmount,
            amountOutMinimum: minAmountOut
        });

The absence of a deadline parameter (deadline: ...) leaves the trade vulnerable to manipulation. An attacker could exploit this by submitting transactions at opportune times, causing the trade to execute under unfavorable market conditions. Advanced protocols like Automated Market Makers (AMMs) can allow users to specify a deadline parameter that enforces a time limit by which the transaction must be executed. Without a deadline parameter, the transaction may sit in the mempool and be executed at a much later time potentially resulting in a worse price for the user.

Scenerio

Alice initiates a transaction using the getZetaFromEth function with a substantial amount of WETH. The function parameters include minAmountOut and destination address, crucial for Alice's expected outcome. Malicious miners, aware of the vulnerability, observe Alice's pending transaction. They strategically choose when to include Alice's transaction in a block, considering market conditions. A miner, Mallory, front-runs Alice's transaction by submitting a transaction with a higher gas fee. Mallory's transaction gets prioritized, and the trade executes under Mallory's chosen market conditions. The market conditions chosen by Mallory lead to increased slippage, resulting in Alice receiving fewer ZetaTokens than expected. Due to the absence of a deadline parameter, Alice's trade becomes vulnerable to manipulation.

NB

AMM users should be able to set expiration deadlines and slippage parameters to prevent potential critical loss of funds vulnerabilities.

Tools Used

https://docs.uniswap.org/contracts/v3/guides/swaps/single-swaps and https://github.com/Uniswap/v3-periphery/blob/main/contracts/interfaces/ISwapRouter.sol

Recommended Mitigation Steps

In order to fix a security vulnerability, it is suggested that a deadline parameter be added to ExactInputSingleParams struct and ExactInputParams struct. This parameter should be used in the code to make sure that trades are completed within a set timeframe. By doing this, users will be protected from possible exploits.

Assessed type

MEV

c4-pre-sort commented 10 months ago

DadeKuma marked the issue as duplicate of #122

c4-pre-sort commented 10 months ago

DadeKuma marked the issue as insufficient quality report

c4-judge commented 9 months ago

0xean changed the severity to QA (Quality Assurance)