0xRstStn - Some Tokens (e.g. USDT) will revert on IERC20 ```approve```

[sherlock-admin2]

0xRstStn

medium

Some Tokens (e.g. USDT) will revert on IERC20 approve

Summary

Some tokens like USDT will revert when invoking IERC20.approve. When approve is invoked into USDT, it:

• does not return any value
• when allowance is not zero, allowance has to be set to zero before assigning a new value. Otherwise, it will revert.

Vulnerability Detail

Function _transferTokens in ChilizWrapperFactory.sol and function _approveAndWrap in JalaMasterRouter.sol invoke IERC20(token).approve(). In case the underlying token used is USDT, this functions will revert so USDT or any token with a similar behavior cannot be used.

Impact

USDT or any token with similar behavior cannot be used as it will revert when invoking approve. Run this PoC on Foundry showing that IERC20.approve reverts when used with USDT. Need to set MAINNET_RPC_URL on .env

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

import "forge-std/Test.sol";
import "../contracts/interfaces/IERC20.sol";

contract USDTFailure is Test {
    string MAINNET_RPC_URL = vm.envString("MAINNET_RPC_URL");
    address USDT = 0xdAC17F958D2ee523a2206206994597C13D831ec7;

    function setUp() public {
        uint256 mainnetFork = vm.createFork(MAINNET_RPC_URL);
        vm.selectFork(mainnetFork);
        assertEq(vm.activeFork(), mainnetFork);
        deal(USDT, address(this), 200 ether, true);
    }

    function testFailIERC20Approve() public {
        // It will revert as no value is returned
        IERC20(USDT).approve(address(1), 100 ether);
    }

    function testUSDTApprove() public {
        // Check that it does not return any value
        (bool success, bytes memory data) =
            USDT.call(abi.encodeWithSignature("approve(address,uint256)", address(1), 100 ether));
        require(success, "Tx failed");

        assertEq(IERC20(USDT).allowance(address(this), address(1)), 100 ether);
        assertEq(data.length, 0);
        console.logBytes(data);

        // Once the allowance is not zero, it has to be set to zero before setting a new vaule
        vm.expectRevert();
        USDT.call(abi.encodeWithSignature("approve(address,uint256)", address(1), 100 ether));
        require(success, "Tx failed");

        // allowance set to zero and then to a new value
        USDT.call(abi.encodeWithSignature("approve(address,uint256)", address(1), 0 ether));
        require(success, "Tx failed");
        USDT.call(abi.encodeWithSignature("approve(address,uint256)", address(1), 200 ether));
        require(success, "Tx failed");
        assertEq(IERC20(USDT).allowance(address(this), address(1)), 200 ether);
    }
}

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/utils/ChilizWrapperFactory.sol#L69-L74

https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaMasterRouter.sol#L328-L331

Tool used

Foundry

Recommendation

Use OZ SafeERC20.sol library or similar and invoke safeIncreaseAllowance.

Duplicate of #117