code-423n4 / 2022-08-frax-findings

2 stars 1 forks source link

Not calling approve(0) before setting a new approval might cause reverts when used with Tether (USDT) #322

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L1103 https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L1184

Vulnerability details

Impact

Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s approve() function will revert if the current approval is not zero, to protect against front-running changes of approvals.

The FraxlendPairCore contract as currently implemented does not handle these sorts of tokens properly when they are used with swapper in leveragedPosition and repayAssetWithCollateral function which might result in reverts.

Proof of Concept

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to consider using OpenZeppelin's SafeERC20's:

token.safeApprove(0);
token.safeApprove(_address, _amount);
amirnader-ghazvini commented 2 years ago

Duplicate of #19