sherlock-audit / 2023-12-flatmoney-judging

11 stars 9 forks source link

jennifer37 - improper priceDiff usage when offchain price is invalid #106

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

jennifer37

medium

improper priceDiff usage when offchain price is invalid

Summary

improper priceDiff usage when offchain price is invalid

Vulnerability Detail

In OracleModule::_getPrice(), we have two price, on-chain price from Chainlink and off-chain price from Pyth. And we will make sure price difference between on-chain price and off-chain price is less than maxDiffPercent. The maxDiffPercent check should check price difference on condition that both prices are valid.

For example, there is something wrong in Pyth price, and has told us offchainInvalid is true. In this scenario, offchainPrice value can be any value, we should not use this value to do anything. It's possible to revert via FlatcoinErrors.PriceMismatch error. In this case, we should make use of on-chain price only.

Impact

If offchian price is invalid, it could cause revert even if we have one good on-chain price.

Code Snippet

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

Tool used

Manual Review

Recommendation

If offchainInvalid is true, skip maxDiffPercent check.

Duplicate of #177

sherlock-admin commented 8 months ago

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

takarez commented:

valid: the misMatch error should be skipped in case of offChainInvalid returned true; medium(2)

sherlock-admin commented 8 months ago

The protocol team fixed this issue in PR/commit https://github.com/dhedge/flatcoin-v1/pull/270.