IllIllI - Positions cannot be liquidated once the oracle prices are zero

[sherlock-admin]

IllIllI

medium

Positions cannot be liquidated once the oracle prices are zero

Summary

In the unlikely event that the price of an asset reaches zero, there is no way to liquidate the position, because both usages of oracles will revert.

Vulnerability Detail

Zero is treated as an invalid value, for both index oracle prices, as well as position prices.

Impact

Positions won't be liquidatable, at an extremely critical moment that they should be liquidatable. Losses and fees will grow and the exchange will become insolvent.

Code Snippet

The Chainlink oracle rejects prices of zero:

// File: gmx-synthetics/contracts/oracle/Oracle.sol : Oracle._setPricesFromPriceFeeds()   #1

571                (
572                    /* uint80 roundID */,
573                    int256 _price,
574                    /* uint256 startedAt */,
575                    /* uint256 timestamp */,
576                    /* uint80 answeredInRound */
577                ) = priceFeed.latestRoundData();
578    
579                uint256 price = SafeCast.toUint256(_price);
580                uint256 precision = getPriceFeedMultiplier(dataStore, token);
581    
582                price = price * precision / Precision.FLOAT_PRECISION;
583    
584                if (price == 0) {
585 @>                 revert EmptyFeedPrice(token);
586                }
587:   

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/oracle/Oracle.sol#L571-L587

As does the usage of off-chain oracle prices:

// File: gmx-synthetics/contracts/oracle/Oracle.sol : Oracle.getLatestPrice()   #2

346            if (!secondaryPrice.isEmpty()) {
347                return secondaryPrice;
348            }
349    
350            Price.Props memory primaryPrice = primaryPrices[token];
351            if (!primaryPrice.isEmpty()) {
352                return primaryPrice;
353            }
354    
355 @>         revert OracleUtils.EmptyLatestPrice(token);
356:       }

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/oracle/Oracle.sol#L341-L356

Tool used

Manual Review

Recommendation

Provide a mechanism for positions to be liquidated even if the price reaches zero

[xvi10]

oracles should report the minimum possible price for this case

[IllIllI000]

@xvi10 if the price drops from $100 down to zero, isn’t that going to cause problems? If you don’t want to work with prices of zero, shouldn’t there be, e.g. 1 wei of value so that wildly incorrect prices aren’t used?

[xvi10]

i think if we allow zero an issue could be raised that allowing zero increases the risk that oracle malfunctions leading to reports of zero prices could cause losses to the pool

in that case, perhaps the recommendation in the previous comment is not correct, the oracle should report zero as the price but the market would need some manual intervention to check and settle

[IllIllI000]

the readme states that Oracle signers are expected to accurately report the price of tokens, so any misreporting would be a separate bug, and reporting a price of 100 seems much more dangerous than an unidentified bug. As is described in #155, using a sentinel value would be safer

[xvi10]

agree on that, i'm not sure if it is worth the complexity to handle a zero price case vs manually settling the market, will mark this as confirmed and may not fix

some calculations such as position.sizeInTokens would not make sense if the price is zero or negative so I believe the contracts in the current state cannot support these values and the markets would need to be manually settled

[hack3r-0m]

Escalate for 10 USDC

This issue should be invalid, because of how unlikely this is to be happen. A same argument (with similiar unlikely odds) can be made that positions cannot be liquidated if ethereum goes down as a network.

[sherlock-admin]

Escalate for 10 USDC

This issue should be invalid, because of how unlikely this is to be happen. A same argument (with similiar unlikely odds) can be made that positions cannot be liquidated if ethereum goes down as a network.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[IllIllI000]

There is a viable scenario (even if unlikely) https://docs.sherlock.xyz/audits/judging/judging . This is a valid risk and is easily protected against. CEX securities go to zero all the time, and as crypto grows and ages, the same will become true for DAO tokens

[hrishibhat]

Escalation rejected

Although unlikely, a zero price should not break the liquidations.
Considering this a valid medium

[sherlock-admin]

Escalation rejected

Although unlikely, a zero price should not break the liquidations.
Considering this a valid medium

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.

[xvi10]

this is a valid issue, but as mentioned the market needs to be manually settled as some calculations may not make sense for zero or negative values