ge6a - DOS for long periods of time due to revert in getPrice()
Closed this issue · 2 comments
ge6a
high
DOS for long periods of time due to revert in getPrice()
Summary
The protocol uses the OracleModule to obtain the price of rETH in USD. This module extracts two prices from Chainlink and Pyth, then compares them to catch any potential errors in the oracle results. The issue is that this check is performed before validating the Pyth price, leading to long periods during which the protocol's user functions will be unavailable due to a revert in the Oracle module.
Vulnerability Detail
The function _getOffchainPrice() calls getPriceNoOlderThan on the Pyth oracle, passing the maxAge parameter. This parameter is set by the administrator, and I confirmed with the sponsor that it will likely be set to 25 hours. If there is no price update within the maxAge period, the function reverts.
This revert is caught by a try/catch, with _getOffchainPrice returning invalid=true and price=0.
Therefore, when calculating priceDiff in _getPrice, a price of 0 is used, and the difference will be 100%. This is certainly more than the allowed difference in maxDiffPercent, causing getPrice to revert. The problem is that the check for price difference is done before validating whether the price returned by Pyth is valid.
Checking the Pyth network contract (0x8250f4aF4B972684F7b336503E2D6dFeDeB1487a) at the time of writing this report shows that the rETH price has not been updated since January 12th (23 days).
Flatmoney requires a price update from keepers when executing operations, but this only happens after an announcement for such updates. However, when creating an announcement for a user operation, the getKeeperFee() function is called from the KeeperFee module, which makes a getPrice() request to the Oracle module.
As discussed above, getPrice would revert. I want to note that it is entirely normal to have periods without deposits, withdrawals, or changes in leverage positions, as these are not operations that users frequently perform. Also, the README file does not describe any off-chain mechanism to update the rETH price in the Pyth contract if it is not updated recently.
Impact
DOS on the main user functions for extended periods. This would lead to various financial losses for users, such as being unable to add additional collateral in case of risk of liquidation caused by changes in the rETH price in the global market. I want to note that a liquidator would be able to pass a price update and execute a liquidation.
Code Snippet
Above
Tool used
Manual Review
Recommendation
A couple of ideas:
- Check price difference only if the Pyth network price is valid.
- Create an off-chain bot to update the Pyth network rETH price if needed.
Duplicate of #177
1 comment(s) were left on this issue during the judging contest.
takarez commented:
valid: medium(2)
The protocol team fixed this issue in PR/commit https://github.com/dhedge/flatcoin-v1/pull/270.