Tokens that don't allow to change allowance if it is not set to 0 beforewill not work correcntly with Trading contract

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L643-L659

Vulnerability details

Impact

Because function Trading._handleDeposit every time approves _stableVault contract for max allowance it will work only once for such tokens as usdt.

Proof of Concept

Trading._handleDeposit function is used in Trading contract to get funds from trader.
https://github.com/code-423n4/2022-12-tigris/blob/main/contracts/Trading.sol#L643-L659

    function _handleDeposit(address _tigAsset, address _marginAsset, uint256 _margin, address _stableVault, ERC20PermitData calldata _permitData, address _trader) internal {
        IStable tigAsset = IStable(_tigAsset);
        if (_tigAsset != _marginAsset) {
            if (_permitData.usePermit) {
                ERC20Permit(_marginAsset).permit(_trader, address(this), _permitData.amount, _permitData.deadline, _permitData.v, _permitData.r, _permitData.s);
            }
            uint256 _balBefore = tigAsset.balanceOf(address(this));
            uint _marginDecMultiplier = 10**(18-ExtendedIERC20(_marginAsset).decimals());
            IERC20(_marginAsset).transferFrom(_trader, address(this), _margin/_marginDecMultiplier);
            IERC20(_marginAsset).approve(_stableVault, type(uint).max);
            IStableVault(_stableVault).deposit(_marginAsset, _margin/_marginDecMultiplier);
            if (tigAsset.balanceOf(address(this)) != _balBefore + _margin) revert BadDeposit();
            tigAsset.burnFrom(address(this), tigAsset.balanceOf(address(this)));
        } else {
            tigAsset.burnFrom(_trader, _margin);
        }        
    }

The line that we need to check is IERC20(_marginAsset).approve(_stableVault, type(uint).max);. In this line Trading contract set max allowance for vault contract in marginAsset token.
In case if such token is USDT or similar(that doesn't allow to provide allowance if current allowance was not 0) will be used that means that this function will succeed only one time. On all next calls it will revert.

Because there is no function that allows to set allowance for USDT token to 0 it means that it will not be possible to use USDT anymore.

I believe this is high severity issue as it will fully block ability to use USDT token with Trading contract.

Tools Used

VsCode

Recommended Mitigation Steps

Set allowance to 0 after transfer, or set allowance equal to the value you want to be transferred.

[c4-judge]

GalloDaSballo marked the issue as duplicate of #104

[c4-judge]

GalloDaSballo marked the issue as satisfactory

[c4-judge]

GalloDaSballo changed the severity to 2 (Med Risk)