code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L200

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 transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert.

Proof of Concept

https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L200

        require(IERC20(tokenAddress).transfer(owner, tokenAmount), "recoverERC20: Transfer failed");

Tools Used

Manual Analysis

Recommended Mitigation Steps

it’s highly advised to use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead.

FortisFortuna commented 2 years ago

Not really medium risk. Technically you could use safeTransfer, but if someone were to accidentally send something to this contract, it would most likely be either ETH, FRAX, frxETH, or sfrxETH, all of which are transfer compliant.

joestakey commented 2 years ago

Duplicate of #18

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