code-423n4 / 2023-05-asymmetry-mitigation-findings

2 stars 2 forks source link

Mitigation Confirmed for NEW #9

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Note: Issue has not actually been resolved but for some reason I can't get my issues to submit without "Mitigation confirmed (no new vulnerabilities detected)" checked so I am doing this as a work around

Severity

High

Lines of code

https://github.com/asymmetryfinance/smart-contracts/pull/262/files#diff-e500be4c081e76873145b6e38a37c2d59d6d57bd96cc581c3099582be79ba505R125-R137

Impact

Derivative will become broken and all funds lost even if there is a slight depeg

Proof of Concept

    uint256 oraclePrice = IFrxEthEthPool(FRX_ETH_CRV_POOL_ADDRESS)
        .price_oracle();
    uint256 priceDifference;
    if (oraclePrice > 10 ** 18) priceDifference = oraclePrice - 10 ** 18;
    else priceDifference = 10 ** 18 - oraclePrice;
    require(priceDifference < 10 ** 15, "frxEth possibly depegged"); // outside of 0.1% we assume depegged

    uint256 frxEthAmount = IsFrxEth(SFRX_ETH_ADDRESS).convertToAssets(

The new ethPerDerivitive function in sfrxEth.sol will revert if the oracle price returns outside of 0.1% of the 1:1 peg. This can be extremely problematic given how integrated this function is.

If the price of the derivative ever settles outside of the peg the funds in the contract will be permanently stuck. FRX_ETH_CRV_POOL_ADDRESS is a constant that can't be updated and ethPerDerivative is called inside the withdraw function. Coupled with the fact that sfrxETH has no emergency recovery function. The funds will be stuck forever in that case.

Minor and consistent depegs (similar to how LUSD is typically valued at $1.01) can happen for any number of reasons. The major issue is that if that happens all fund will be completely unrecoverable.

Tools Used

Manual Review

Recommended Mitigation Steps

The curve oraclePrice is already an EMA and not the current price of the pool so it is safe to use it as an oracle price. If unwilling to do this then there should at least be an emergency eject function to save the funds.

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #58