Owner can rug the submitted Ether in `frxETHMinter.sol`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/8073cc4ca44d9162873494f1cd9915a5d7b46f2b/src/frxETHMinter.sol#L191

Vulnerability details

Proof of concept

In frxETHMinter.sol there is the following code

function recoverEther(uint256 amount) external onlyByOwnGov {
        (bool success,) = address(owner).call{ value: amount }("");
        require(success, "Invalid transfer");

        emit EmergencyEtherRecovered(amount);
    }

This allows the owner of the contract (bypassing timelock) to withdraw all of the submitted Ether to the contract before it was deposited to the ETH 2.0 deposit contract, esentially rugging everyone who submitted Ether to mint frxETH.

Impact

The impact of this issue is that since the protocol is ruggable, it’s reputation will suffer. Also if this is exploited then all of the users will essentially lose all of their submitted Ether

Recommendation

Remove the recoverEther functionality completely.

[FortisFortuna]

We are well aware of the permission structure. The owner will most likely be a large multisig. We mentioned the Frax Multisig in the scope too.

[joestakey]

Duplicate of #107