Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

frxETH off-peg handling #223

Closed
code423n4 opened this issue Sep 25, 2022 · 2 comments
Closed

frxETH off-peg handling #223

code423n4 opened this issue Sep 25, 2022 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate This issue or pull request already exists frxETH off-peg in discussion Discussion about this issue is ongoing, and not yet resolved. sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@code423n4
Copy link
Contributor

Lines of code

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

Vulnerability details

Impact

There is no guarantee that the validators will not lose fund. There are multiple pitfalls can result in loss, according to Ethereum document: penalties could come from slashing, inactivity leak, reference. Or even unexpected bugs can cause loss to the validators, such as this news:
Seventy-five Eth2 validators got slashed this week due to a bug witnessed by Staked.

If some validators did incur losses, or lots of the validators suffer loss due to some softwares bugs, the equity of the frxETH holders could be less than what they have deposited. Then the booking value of the frxETH can not support the 1:1 peg, off-peg could be triggered, the consequence might go out of control. Because at the same time, the multisig is still sending staking rewards ETH to the frxETH contract to mint with hardcoded 1:1 exchange ratio. For the sfrxETH holders, what they receive is much over valued. To stop loss, they will tend to exit as soon as possible. If this scenario happens, it is expected to have large scale withdraw() of the vault, followed by panic sell off, and eventually even further depreciation of frxETH and market collapse.

If the vault was withdraw() to empty, the potential issue of ERC4626 vault exchange rate manipulation could also happen, further harm the vault and other users.

This issue is deemed as High because once it happens, the impacts might be catastrophic to the protocol.

Proof of Concept

The exchange rate of frxETH to ETH is hardcoded. During off-peg period, the incorrect ratio could enhance the market tumble.

// src/frxETHMinter.sol
    function submitAndDeposit(address recipient) external payable returns (uint256 shares) {
        // ...
        uint256 sfrxeth_recieved = sfrxETHToken.deposit(msg.value, recipient);
        // ...
    }

Tools Used

Manual analysis.

Recommended Mitigation Steps

  • monitor the under-performed validators and stop loss timely.
  • maybe change the hardcoded frxETH to ETH exchange ratio, and allow for some variation. Then market trades might help to stabilize the price.
  • buy insurance as downside protection for off-peg, for potential large scale validator loss. Otherwise, no base booking value could result in disastrous results like UST/luna.
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Sep 25, 2022
code423n4 added a commit that referenced this issue Sep 25, 2022
@FortisFortuna FortisFortuna added in discussion Discussion about this issue is ongoing, and not yet resolved. frxETH off-peg labels Sep 26, 2022
@FortisFortuna
Copy link

From @denett
Nobody will mint frxETH at 1-1 when the peg is off, they will just buy on Curve. After unlocks we will use the ETH to buy back frxETH from the market. If a huge slashing event happend and we will only have 0.80 ETH per frxETH the frxETH price will be below 0.8 ETH on Curve, so we will then also buy back below the peg.
We can chose to restore the peg by migrating the frxETH token to frxETHv2 that is pegged 1-1 to ETH again.

@FortisFortuna FortisFortuna added disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons labels Sep 26, 2022
@0xean
Copy link
Collaborator

0xean commented Oct 13, 2022

dupe of #113

@0xean 0xean closed this as completed Oct 13, 2022
@0xean 0xean added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value duplicate This issue or pull request already exists and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) duplicate This issue or pull request already exists frxETH off-peg in discussion Discussion about this issue is ongoing, and not yet resolved. sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

3 participants