search

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

0 stars 0 forks source link

Direct WETH swap fails due to incompatibility with ``ZetaTokenConsumerUniV3`` & ``ZetaTokenConsumerPancakeV3`` #422

Open c4-bot-4 opened 11 months ago

c4-bot-4 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol#L98-L122 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol#L158-L182

Vulnerability details

Impact

User use the ZetaTokenConsumerUniV3.sol to get the zeta tokens. Either through ETH or through other ERC20 tokens. One of the most important tokens in Defi today is the WETH token, all swaps in the codebase to/from ZETA are routed through the ZETA/WETH pool. However this contact, as well as the ZetaTokenConsumerPancakeV3 and the ZetaTokenConsumerTrident do not permit users to swap WETH directly to ZETA. This is because of an oversight in the swap function that require the existence of a WETH/WETH pool. As seen below, when the input/output token is WETH, the function will revert: 

 function getZetaFromToken(
        address destinationAddress,
        uint256 minAmountOut,
        address inputToken,
        uint256 inputTokenAmount
    ) external override returns (uint256) {
        if (destinationAddress == address(0) || inputToken == address(0)) revert ZetaCommonErrors.InvalidAddress();
        if (inputTokenAmount == 0) revert InputCantBeZero();

        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
        });

Proof of Concept

From the Pancake V3 factory and Uni V3 factory contracts, it is evident that pools cannot be created for a token with itself. Therefore users cannot swap WETH to WETH thereby breaking the path needed to swap WETH to Zeta or vice versa. In the code base, the issue is present:

Tools Used

Manual Review

Recommended Mitigation Steps

Implement a check to see if the input/output token is WETH, if so, perform the swap directly between WETH and ZETA without a second swap.

Assessed type

Other

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

DadeKuma commented 10 months ago

This seems false:

This is because of an oversight in the swap function that require the existence of a WETH/WETH pool.

And the poc is insufficient as it's just a list of code references.

Josephdara commented 10 months ago

This is not false, all references stated in the POC section are lines of code in the codebase within scope that require a WETH/WETH pool.

When the Input token is WETH, it attempts to first swap to WETH, then to ZETA. When the output token is WETH, it first swaps ZETA to WETH, then attempts swapping WETH to WETH.

This was explained above with all the points that this happened listed in the POC section .

As at the time of this submission, there was no requirement for a compulsory coded POC but this can be provided.

Still, I leave this to the judge. Thanks

0xean commented 10 months ago

more evidence would certainly have been welcomed, but I do follow the wardens belief that this would cause an issue when attempting to call getZetaFromToken() and the input is WETH. Reaching out to sponsor for comment (@lumtis )

c4-judge commented 10 months ago

0xean removed the grade

c4-judge commented 10 months ago

0xean marked the issue as satisfactory

c4-sponsor commented 10 months ago

lumtis (sponsor) confirmed

c4-judge commented 10 months ago

0xean marked the issue as selected for report