Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerUniV3.strategy.sol#L108 https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/protocol-contracts/contracts/evm/tools/ZetaTokenConsumerPancakeV3.strategy.sol#L136 https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code#L199

Vulnerability details

Impact

Consumer contracts allow users to swap ZETA tokens acting as an intermediate. First, the user sends tokens to the consumer, then the consumer approves them to the Uniswap router and makes a swap sending exchanged tokens back to the user.

    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(pancakeV3Router), inputTokenAmount);

Unfortunately, some tokens, such as USDT, require the allowance to be set to zero first. If this is not done, operations with this token will be blocked. https://etherscan.io/token/0xdac17f958d2ee523a2206206994597c13d831ec7#code#L199

    function approve(address _spender, uint _value) public onlyPayloadSize(2 * 32) {

        // To change the approve amount you first have to reduce the addresses`
        //  allowance to zero by calling `approve(_spender, 0)` if it is not
        //  already 0 to mitigate the race condition described here:
        //  https://github.com/ethereum/EIPs/issues/20#issuecomment-263524729
        require(!((_value != 0) && (allowed[msg.sender][_spender] != 0)));

Proof of Concept

How is this possible? Didn't the consumer spend all it's allowance in the previous swap? There may be a situation when a swap is partially executed by the uniswap pool, for instance there was not enough liquidity. https://github.com/Uniswap/v3-core/blob/main/contracts/UniswapV3Pool.sol#LL640C9-L650C15

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

Therefore only some amount of allowance will be spent https://github.com/Uniswap/v3-periphery/blob/main/contracts/SwapRouter.sol#L72

In this case, the allowance for USDT will be > 0 at the time the next swap transaction arrives, causing that transaction to be cancelled.

Tools Used

Manual review

Recommended Mitigation Steps

Set allowance to 0 before calling approve.

Assessed type

DoS

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

USDT swaps may be blocked due to "approve zero first" requirement #58