c4-bot-8 commented 11 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-L120 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L138-L145 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L162-L172 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L196-L203 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerTrident.strategy.sol#L84-L93 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerTrident.strategy.sol#L121-L130 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerTrident.strategy.sol#L150-L159 https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerTrident.strategy.sol#L188-L197

Vulnerability details

Impact

Loss of fund to sandwich attack at maximum slippage due to deadline protection.

Proof of Concept

The main issue is that Pancake and Trident are both forks of Uniswap AMM WITHOUT the deadline protection.

The deadline property according to Uniswap docs is "the unix time after which a swap will fail, to protect against long-pending transactions and wild swings in prices" ref: https://docs.uniswap.org/contracts/v3/guides/swaps/single-swaps#swap-input-parameters

During wild swing in prices miners can decide to save transactions like this for sometime till the transactions incurs maximum slippage loss and MEV gain for the miner through front-running bots.

Below is the exactInput function of uniswap that has deadline protection through the checkDeadline modifier

/// @inheritdoc ISwapRouter
    function exactInput(ExactInputParams memory params)
        external
        payable
        override
        checkDeadline(params.deadline)
        returns (uint256 amountOut)
    {

And below is Pancakeswap's exactInput function without the deadline protection

/// @inheritdoc IV3SwapRouter
    function exactInput(ExactInputParams memory params) external payable nonReentrant override returns (uint256 amountOut) {

The deadline allows users pass the unix literal time in the future where the transaction should revert.

This will protect the users from keeping their swap transactions "for later" till it incurs maximum slippage loss benefiting frontrunning bots.

Tools Used

Manaul review

Recommended Mitigation Steps

Implement the checkDeadline modifier in the ZetaTokenConsumerTrident.strategy.sol and the ZetaTokenConsumerTrident.strategy.sol contracts and add the checkDeadline modifier to the swap function in order to add deadline protection for swap transactions so that transaction is reverted when deadline is exceeded.

modifier checkDeadline(uint256 deadline) {
        require(block.timestamp <= deadline, 'Transaction too old');
        _;
    }

Then in for example getTokenFromZeta function you add the checkDeadline modifier and accept deadline as input literal

function getTokenFromZeta(
        address destinationAddress,
        uint256 minAmountOut,
        address outputToken,
        uint256 zetaTokenAmount,
++      uint256 deadline
    ) external 
override 
nonReentrant 
++ checkDeadline(deadline)
returns (uint256) {

Assessed type

MEV

c4-pre-sort commented 11 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 10 months ago

0xean changed the severity to QA (Quality Assurance)