[M3] It is impossible to recover stucked non complying ERC-20 tokens

[code423n4]

Lines of code

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

Vulnerability details

Impact

Contract frxETHMinter is unable to recover tokens like USDT

PoC

Tokens that return void on transfer, that is, those who do not follow ERC20 standard will revert when you try to assign the output to a boolean variable. This is the case in you function recoverERC20

Recommended

I believe that it is better just to remove require and check balance off-chain. Otherwise, you will need libraries like SafeTransfer but I don't think that this is really neccessary. Tokens return false generally when you try to transfer more balance that you have or you are not approved.

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

[FortisFortuna]

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]

Duplicate of #18