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

2 stars 1 forks source link

ERC20.approve can be error prone. It is known issue. ERC20.sol could not be safe. #317

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

ERC20 - approve is not safe.

Proof of Concept

For approve, the entire contract depend on ERC20.approve()

_assetContract.approve(_swapperAddress, _borrowAmount);

_collateralContract.approve(_swapperAddress, _collateralToSwap);

Refer following articles for this issue vector. There are suggestions also to resolve it.

https://docs.google.com/document/d/1YLPtQxZu1UAvO9cZ1O2RPXBbT0mooh4DYKjA_jp-RLM/edit

https://docs.openzeppelin.com/contracts/2.x/api/token/erc20#IERC20-approve-address-uint256-:~:text=Beware%20that%20changing,20%23issuecomment%2D263524729

Tools Used

Manual code review.

Recommended Mitigation Steps

It is recommended to use SafeERC20 - safeIncreaseAllowance.

Infact, the contract is using the ERC20.sol, It could be better to use SafeERC20.sol

amirnader-ghazvini commented 2 years ago

Duplicate of #19