Psyduck - Discrepancy between keeper and oracle module staleness periods can result in unexpected reverts and unfair liquidations

[sherlock-admin]

Psyduck

medium

Discrepancy between keeper and oracle module staleness periods can result in unexpected reverts and unfair liquidations

Summary

Discrepancy between keeper and oracle module staleness periods can result in unexpected reverts and unfair liquidations

Vulnerability Detail

Various critical functions in the protocol (e.g. announce functions in the delayed order contract) rely on the both the keeper fee and the oracle module to deliver fresh chainlink price feeds. Let's take a look at both of them

the keeper fee contract uses chainlink price to calculate to keeper's fee shown here

 function getKeeperFee() public view returns (uint256 keeperFeeCollateral) {
        uint256 ethPrice18;
        {
            (, int256 ethPrice, , uint256 ethPriceupdatedAt, ) = _ethOracle.latestRoundData();
            if (block.timestamp >= ethPriceupdatedAt + _STALENESS_PERIOD) revert FlatcoinErrors.ETHPriceStale();

https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/misc/KeeperFee.sol#L81-L85

note the check made to see if the feed isn't stale

  if (block.timestamp >= ethPriceupdatedAt + _STALENESS_PERIOD) revert FlatcoinErrors.ETHPriceStale();

uint256 private constant _STALENESS_PERIOD = 1 days;

Now let's take a look at the oracle module

https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/OracleModule.sol#L141-L149

Just like the keeper fee contract , there is a check to make sure that the feed isn't stale.

   if (block.timestamp > timestamp + onchainOracle.maxAge)
            revert FlatcoinErrors.PriceStale(FlatcoinErrors.PriceSource.OnChain);

Also, take a look at the config to see that the maxage (which is incorrect) would look like

https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/scripts/deployment/configs/OracleModule.config.js#L6

The issue here is that both the keeper fee and the oracle module's staleness periods are not consistent. This means that there can be situations where a feed is too stale for the oracle module but fine for the keeper. This mismatch will ultimately result in reverts in some situations which can be devastating for users as some will not have a chance to shore up their position and avoid unfair liquidations when a price feed becomes fresh enough for both contracts (keeper and oracle)

Impact

Critical protocol functions will revert at times where it shouldn't, potentially causing unfair liquidations for users

Code Snippet

https://github.com/sherlock-audit/2023-12-flatmoney/blob/bba4f077a64f43fbd565f8983388d0e985cb85db/flatcoin-v1/src/OracleModule.sol#L148

Tool used

Manual Review

Recommendation

Make sure that the staleness period for both the keeper fee and the oracle modules are the same

[sherlock-admin]

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

takarez commented:

invalid: POC ?

[nevillehuang]

Invalid, out of scope given issue arises from speculation via configuration files.