Steadefi

Vulnerability Details

In order to check validity of chainlink price for current round, ChainlinkARBOracle contract fetches current and previous rounds.
Then current round is checked to be not stale. In case if it's not, then we have another bunch of checks.

_chainlinkIsBroken function checks that both responds from chainlink are valid, which means that it just check if returned values from chainlink are not weird. And then if that's fine, then 2 responds are checked for price deviation.

https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/oracles/ChainlinkARBOracle.sol#L142-L157

Each token has it's own price deviation and this function checks that change of price from previous respond to current respond is not bigger than max deviation.

The problem is that in case if previous response was stale, then this check can be incorrect and it can't be used to understand if deviation is fine. This is because previous price is provided for older time(which means that real price that was before can be other: bigger or smaller), while deviation check mechanism in this contract should check 2 valid responses. In such case better approach will be to revert and wait for the next chainlink answer, when you have 2 non stale responds.

Impact

Price deviation check can be incorrect and may not work as designed.

Tools Used

VsCode

Recommendations

You need to check if previous response from chainlink was not stale. If it was, then better to revert as deviation check may provide incorrect results.