sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

Bjorn_Bug - Incorrect Validation Of latestRoundData Function Can Lead to Stale Price Issue. #157

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

Bjorn_Bug

medium

Incorrect Validation Of latestRoundData Function Can Lead to Stale Price Issue.

Summary

When OracleModule#_getOnchainPrice() fetches the price from Chainlink, it misses a very important check recommended by the Chainlink protocol to ensure the data is fresh and accurate.

Vulnerability Detail

The _getOnchainPrice() function depends on Chainlink's latestRoundData() but does not verify round completeness. The protocol developers (according to Discord chat) plan to set onchainOracle.maxAge to 24 hours and 10 seconds. This means that the price is considered not stale only when block.timestamp <= updatedAt (Last Time the price was updated) + onchainOracle.maxAge (24H + 10 seconds).

(, int256 _price, , uint256 updatedAt, ) = oracle.latestRoundData();
timestamp = updatedAt;
if (block.timestamp > timestamp + onchainOracle.maxAge) 
    revert FlatcoinErrors.PriceStale(FlatcoinErrors.PriceSource.OnChain);

Impact

Chainlink could return stale data, which the protocol might accept when the keeper execute pending orders or liquidate, even though it primarily relies on Pyth's offchain price. However, if offchainInvalid == true, the on-chain price returned might not be accurate.

This issue for example could result to the liquidation of users who should not be liquidated when the canLiquidate function is called.

Code Snippet

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

Tool used

Manual Review

Recommendation

Add the following checks and reply on the returned value from latestRoundData to verify for price stale instead of onchainOracle.maxAge set by the protocol admin.


(uint80 roundId , int256 _price, , uint256 updatedAt, uint80 answeredInRound) = oracle.latestRoundData();

require(answeredInRound >= roundId, "Price Stale");

Duplicate of #3

sherlock-admin commented 8 months ago

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

takarez commented:

invalid