anti-flashloan mechanism may lead to protocol default

[code423n4]

Handle

gzeon

Vulnerability details

Impact

There is a price check to avoid flash loan attacks which significantly moved the price. If current price is 5% lower than the stored twap price, the liquidation will fail. This design can be dangerous as it is to openleverage's benefit to close under-collateralized position ASAP when there is a huge market drawdown. When the market keep trading downward, it is possible that the spot price keep trading 5% lower than the twap, which prevent any liquidation from happening and causing the protocol to be under-collateralized.

Proof of Concept

https://github.com/code-423n4/2022-01-openleverage/blob/501e8f5c7ebaf1242572712626a77a3d65bdd3ad/openleverage-contracts/contracts/OpenLevV1Lib.sol#L191

            // Avoid flash loan
            if (prices.price < prices.cAvgPrice) {
                uint differencePriceRatio = prices.cAvgPrice.mul(100).div(prices.price);
                require(differencePriceRatio - 100 < maxLiquidationPriceDiffientRatio, 'MPT');
            }

Recommended Mitigation Steps

Instead of revert with maxLiquidationPriceDiffientRatio, use the twap price to determine if the position is healthy.

[0xleastwood]

From first impression, this findings seems legitimate. Can I get some more details on why it was disputed? @ColaM12

[ColaM12]

There is always a chance to front run a flash loan transaction before trading in OpenLev. Also, see in line 196, position is considered not healthy only if all three price check failed including the twap price.

[0xleastwood]

It looks like only one condition would need to be satisfied for isPositionHealthy to return false as it uses || and not &&.

[ColaM12]

Do you mean return true? All 3 price checks should fail when liquidating. But the position may still hold funds to pay off debt. by using maxLiquidationPriceDiffientRatio, under-priced-swaps can be limited . Otherwise, all remaining funds in the position could be drained from a flash loan attack which directly leads to a bad debt to lender.

[0xleastwood]

Ahh sorry my mistake. I misinterpreted that.

[0xleastwood]

I agree with the sponsor here. The issue outlined by the warden seems to be safeguarded by the two other checks in isPositionHealthy()

[0xleastwood]

Actually thinking about this more, I think the warden raised an issue related to the liquidations continuing to fail if the price keeps trending downward at an accelerated pace. I don't think the protocol would be able to respond to such events if this reverts.

[0xleastwood]

After discussion with the sponsor, we have agreed that this issue is valid. It is expected that the TWAP is only valid for 1 min. By removing this condition, there is potential for even larger security issues. So the sponsor has decided to make this a wont-fix but I'll keep the issue open as it is valid.

[0xleastwood]

This was an awesome find!