code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-y2k-finance/blob/ac3e86f07bc2f1f51148d2265cc897e8b494adf7/src/oracles/PegOracle.sol#L46-L83

Vulnerability details

Impact

Insurance is to protect the user in case the pegged asset drops significantly below the underlying but risk users are required to payout if the pegged asset is worth more than the underlying

Proof of Concept

    if (price1 > price2) {
        nowPrice = (price2 * 10000) / price1;
    } else {
        nowPrice = (price1 * 10000) / price2;
    }

The above lines calculates the ratio using the lower of the two prices, which means that in the scenario that the pegged asset is worth more than the underlying, a depeg event will be triggered. This is problematic for two reasons. The first is that many pegged assets are designed to maintain at least the value of the underlying. They put very strong incentives to keep the asset from going below the peg but usually use much looser policies to bring the asset down to the peg, since an upward break from the peg is usually considered benign. The second is that when a pegged asset move above the underlying, the users who are holding the asset are benefiting from the appreciation of the asset; therefor the insurance is not needed.

Because of these two reasons, it is my opinion that sellers would demand a higher premium from buyers as a result of the extra risk introduced by the possibility of having to pay out during an upward depeg. It is also my opinion that these higher premiums would push users seeking insurance to other cheaper products that don't include this risk

Tools Used

Recommended Mitigation Steps

The ratio returned should always the ratio of the pegged asset to the underlying (i.e. pegged/underlying)

MiguelBits commented 2 years ago

dup https://github.com/code-423n4/2022-09-y2k-finance-findings/issues/26

HickupHH3 commented 2 years ago

not a duplicate.

Pegged tokens go both ways: either valued more or less than the asset it's pegging to (underlying token).

The warden is arguing that when the pegged token is worth more than the underlying (eg. worth > $1 for a stablecoin), the users are still eligible for a payout, which he argues shouldnt be the case.

I agree with the warden; from experience, most projects see it as a positive if their algo stablecoin is worth more than the underlying, and so, wouldn't do nothing about it. In fact, they'd probably use it as a shilling point to attract more users to mint more of these tokens to help bring the price down. This scenario should not be covered by the insurers.

HickupHH3 commented 2 years ago

Agree with rationale of high severity provided by #419:

Setting the severity to be high as that's the violation of core logic with substantial fund loss for all hedge users.

Between this and #419, it's hard to decide which one to be selected for the report. I like how #419 provides numbers to guide the reader. However, this issue gives good reasoning as to why only the "negative" de-peg should be covered.

Ideally, I would select both, but the stronger reasoning edges out.

code-423n4 / 2022-09-y2k-finance-findings