Trading#_handleDeposit won't work with USDT and other tokens that require zero allowance before approval

[code423n4]

Lines of code

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

Vulnerability details

Impact

USDT and other tokens that require zero allowance before approval will be incompatible as a margin asset

Proof of Concept

IERC20(_marginAsset).transferFrom(_trader, address(this), _margin/_marginDecMultiplier);
IERC20(_marginAsset).approve(_stableVault, type(uint).max);
IStableVault(_stableVault).deposit(_marginAsset, _margin/_marginDecMultiplier);

Trading#_handleDeposit is used to pull marginAsset from the trader and deposit into the correct vault. Before each deposit it approves the marginAsset to the stableVault then deposits. This is problematic for assets like USDT, which require the current allowance to be zero. After the first deposit, all following deposits for USDT will revert when trying to make the approve call.

Tools Used

Manual Review

Recommended Mitigation Steps

    IERC20(_marginAsset).transferFrom(_trader, address(this), _margin/_marginDecMultiplier);
-   IERC20(_marginAsset).approve(_stableVault, type(uint).max);
+   IERC20(_marginAsset).approve(_stableVault, _margin/_marginDecMultiplier);
    IStableVault(_stableVault).deposit(_marginAsset, _margin/_marginDecMultiplier);

[c4-judge]

GalloDaSballo marked the issue as duplicate of #198

[c4-judge]

GalloDaSballo marked the issue as satisfactory