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

2 stars 1 forks source link

frxETHMinter: Non-conforming ERC20 tokens not recoverable #18

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

There is a function recoverERC20 to rescue any ERC20 tokens that were accidentally sent to the contract. However, there are tokens that do not return a value on success, which will cause the call to revert, even when the transfer would have been succesful. This means that those tokens will be stuck forever and not recoverable.

Proof Of Concept

Someone accidentally transfers USDT, one of the most commonly used ERC20 tokens, to the contract. Because USDT's transfer does not return a boolean, it will not be possible to recover those tokens and they will be stuck forever.

Recommended Mitigation Steps

Use OpenZeppelin's safeTransfer.

FortisFortuna commented 1 year 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.

0xean commented 1 year ago

I think this qualifies as a M risk. Sponsor has created functionality to recover erc20 tokens. Wardens have shown a path to which this functionality does not work correctly.

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.