Not calling `approve(0)` before setting a new approval causes the call to revert when used with Tether (USDT)

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L214 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L157

Vulnerability details

Impact

Some tokens (like USDT) do not work when changing the allowance from an existing non-zero allowance value (it will revert if the current approval is not zero to protect against front-running changes of approvals). These tokens must first be approved for zero and then the actual allowance can be approved.

Proof of Concept

• File: RubiconRouter.sol:

  154:     // function for infinite approvals of Rubicon Market
  155:     function approveAssetOnMarket(address toApprove) public {
  156:         // Approve exchange
  157:         ERC20(toApprove).approve(RubiconMarketAddress, 2**256 - 1);
  158:     }

• File: BathToken.sol:

  214:         IERC20(address(token)).approve(RubiconMarketAddress, 2**256 - 1);

Recommended Mitigation Steps

Use approve(RubiconMarketAddress, 0) to set the allowance to zero immediately before each of the existing approve() calls.

[bghughes]

Duplicate of #100