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

2 stars 2 forks source link

Mitigation Confirmed for H-04 #7

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

Medium

Lines of code

https://github.com/asymmetryfinance/smart-contracts/pull/262/files#diff-e500be4c081e76873145b6e38a37c2d59d6d57bd96cc581c3099582be79ba505R124-R140

Impact

Value of sfrxETH is incorrect

Proof of Concept

function ethPerDerivative() public view returns (uint256) {
    // There is no chainlink price fees for frxEth
    // We making the assumption that frxEth is always priced 1-1 with eth
    // revert if the curve oracle price suggests otherwise
    // Theory is its very hard for attacker to manipulate price away from 1-1 for any long period of time
    // and if its depegged attack probably cant maniulate it back to 1-1
    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(
        10 ** 18
    );
    return frxEthAmount;

If the value of sfrxETH is within the deviation threshold (currently 0.1%) then the frxETH is assumed to be worth 1 ETH. This is problematic as those small deviations will still lead to the token be under/over valued which leaks value.

Tools Used

Manual Review

Recommended Mitigation Steps

Regardless of how close the value is to 1 the oracle price should still be used otherwise there will be leak of value:

-   return frxEthAmount;
+   return (frxEthAmount * oraclePrice / 10 ** 18);
c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

c4-judge commented 1 year ago

Picodes marked the issue as nullified