If the oracle from Pyth is down, OracleModule._getPrice will always revert

Summary

[OracleModule._getPrice](https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L106) uses 2 PriceFeeds: onchainOracle from chainlink and offchainOracle from pyth. This function will get the two prices respectively, calculate the diffPercent of the two prices, and ensure that the maxDiffPercent will not be exceeded.

If offchainOracle goes down, the current implementation will keep diffPercent greater than maxDiffPercent. In this way, tx will revert.

Vulnerability Detail

From [OracleModule.config.js](https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/scripts/deployment/configs/OracleModule.config.js#L8-L14), we can see that:

offchainOracle: {
    pyth: "0x5955c1478f0dad753c7e2b4dd1b4bc530c64749f",
    pythPriceFeedId: "0xca80ba6dc32e08d06f1aa886011eed1d77c77be9eb761cc10d72b7d0a2fd57a6",
    pythMaxPriceAge: 90000,
    pythMinConfidenceRatio: 1000,
},
maxDiffPercent: "5000000000000000", //0.5%

Let's look at the code of [_getPrice](https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L107-L113):

File: flatcoin-v1\src\OracleModule.sol
106:     function _getPrice(uint32 maxAge) internal view returns (uint256 price, uint256 timestamp) {
107:         (uint256 onchainPrice, uint256 onchainTime) = _getOnchainPrice(); // will revert if invalid
108:         (uint256 offchainPrice, uint256 offchainTime, bool offchainInvalid) = _getOffchainPrice();
109:         bool offchain;
110: 
111:         uint256 priceDiff = (int256(onchainPrice) - int256(offchainPrice)).abs();
112:         uint256 diffPercent = (priceDiff * 1e18) / onchainPrice;
113:         if (diffPercent > maxDiffPercent) revert FlatcoinErrors.PriceMismatch(diffPercent);
......
136:     }

If offchainOracle goes down, the duration exceeds the defined pythMaxPriceAge (90000s):

L108, _getOffchainPrice() will return (0, 0, false).

L111, priceDiff = onchainPrice

L112, diffPercent = 1e18

L113, the condition diffPercent > maxDiffPercent is met, tx revert.

File: flatcoin-v1\src\OracleModule.sol
163:     function _getOffchainPrice() internal view returns (uint256 price, uint256 timestamp, bool invalid) {
164:         IPyth oracle = offchainOracle.oracleContract;
165:         if (address(oracle) == address(0)) revert FlatcoinErrors.ZeroAddress("oracle");
166: 
167:->       try oracle.getPriceNoOlderThan(offchainOracle.priceId, offchainOracle.maxAge) returns (
168:             PythStructs.Price memory priceData
169:         ) {
......//this case don't need to explain here
184:         } catch {
185:             invalid = true; // couldn't fetch the price with the asked input param
186:         }
187:     }

oracle.getPriceNoOlderThan will [revert if it cannot obtain maxAge data](https://goerli.basescan.org/address/0xf5bbe9558f4bf37f1eb82fb2cedb1c775fa56832#code#F15#L55). The code of L169-184 will not be executed, but L185 will be executed. Then, the price and timestamp to be returned are both 0 because they are not initialized.

However, _getOnchainPrice() still works normally. The price it returns is fresh.

Impact

For time-sensitive tx, such as liquidation, a position that should have been liquidated cannot be liquidated due to this issue.

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/main/flatcoin-v1/src/OracleModule.sol#L107-L113

Tool used

Manual Review

Recommendation

The situation mentioned in the report (offchainPrice = 0) should be considered when comparing diffPercent and maxDiffPercent.

Duplicate of #177

sherlock-admin commented 6 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid