SuperVault's leverageSwap and emptyVaultOperation can become stuck

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/supervaults/contracts/SuperVault.sol#L320-L326
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/supervaults/contracts/SuperVault.sol#L198-L199

Vulnerability details

leverageSwap and emptyVaultOperation can be run repeatedly for the same tokens. If these tokens happen to be an ERC20 that do not allow for approval of positive amount when allowance already positive, both functions can become stuck.

https://github.com/d-xo/weird-erc20#approval-race-protections

In both cases logic doesn't seem to guarantee full usage of the allowance given. If it's not used fully, the token will revert each next approve attempt, which will render the functions unavailable for the token.

While emptyVaultOperation can be cured by emptying the balance and rerun, in the leverageSwap case there is no such fix possible.

Setting severity to medium as this clearly impacts leverageSwap and emptyVaultOperation availability to the users.

Proof of Concept

leverageSwap calls target token for maximum approval of core each time:

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/supervaults/contracts/SuperVault.sol#L320-L326

  ///@param token The leveraged asset to swap PAR for
  function leverageSwap(bytes memory params, IERC20 token) internal {
    (uint256 parToSell, bytes memory dexTxData, uint dexIndex) = abi.decode(
      params,
      (uint256, bytes, uint )
    );
    token.approve(address(a.core()), 2**256 - 1);

Some tokens do not have maximum amount as an exception, simply reverting any attempt to approve positive from positive, for example current USDT contract, L205:

https://etherscan.io/address/0xdac17f958d2ee523a2206206994597c13d831ec7#code

I.e. if leverageSwap be run again with USDT it will revert all the times after the first.

emptyVaultOperation approves core for the whole balance of stablex:

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/supervaults/contracts/SuperVault.sol#L198-L199

    IERC20 par = IERC20(a.stablex());
    par.approve(address(a.core()), par.balanceOf(address(this)));

Recommended Mitigation Steps

Consider adding zero amount approval before actual amount approval, i.e. force zero allowance before current approval.

[gzeoneth]

Having approve(0) first will still revert with USDT because the interface expect it to return a bool but USDT return void. Fund also won't be stuck because it will revert. Judging as Med Risk as function availability could be impacted. Unlike the core protocol, SuperVault can take any token as input and USDT is listed on various lending protocol like AAVE.