Summary

Potential DoS on liquidations and other important functionalities

Vulnerability Details

Upon functionalities like filling orders and liquidations, we calculate the mark price like this:

This function can revert in some cases which would be extremely detrimental to the health of the protocol, especially when liquidating someone. Let's take a look at this particular line:

We do calculations based on the index price and the price impact (either before or after delta, both lines are vulnerable to reverts). Then, we turn the signed number (SD59x18) into an unsigned one. Thus, if the result of the calculation is a negative number, it will revert. For the calculation to turn out negative, cachedIndexPriceX18.mul(priceImpactBeforeDelta) would have to be a negative number which has an absolute value higher than cachedIndexPriceX18. For that to happen, priceImpactBeforeDelta (or afterDelta for the priceAfterDelta calculation) would have to be < -1e18.

For example, imagine index price is 100e18 and the price impact is -1e18 - 1. Then, the calculation would be . This is a negative number that would revert when turned into an unsigned one. The calculation includes dividing by 1e18 due to the mul() implementation.

Now, the only thing needed is for priceImpactBeforeDelta to be lower than -1e18. This is how it gets its value:

If skewis a negative number (very likely) which has an absolute value higher than skewScale by a large enough amount for it to not round down to 1e18, then the above scenario would occur. Imagine skew = - 10e18 - 10 and skewScale is 10e18. Thus, the calculation would be . Essentially, if the skew goes above the skewScale, then this issue is likely to happen which would be detrimental to the protocol as it can DoS liquidations.

Impact

Potential DoS on liquidations and other important functionalities, likelihood is not very high, impact is critical, thus a medium is appropriate.

Tools Used

Manual Review

Recommendations

Do not unsafe cast