Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oracleMultiply / oracleDivide prices may be from different rounds #46

Closed
code423n4 opened this issue Aug 14, 2022 · 1 comment
Closed
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists invalid This doesn't seem right out of scope

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L524
https://github.com/code-423n4/2022-08-frax/blob/b58c9b72f5fe8fab81f7436504e7daf60fd124e3/src/contracts/FraxlendPairCore.sol#L532

Vulnerability details

Impact

When both oracleMultiply and oracleDivide are used, it can happen that the returned oracle results are from different rounds / have a different age. This can lead to situations where an exchange rate is used that never existed. Such a situation is much worse IMO than the (known) risk of outdated exchange rates, because they at least existed at some point in the past. But when one oracle returns a result that is 2 hours old and the other result is 5 minutes old, the calculated exchange rate may have never existed, but is still used.

Recommended Mitigation Steps

Use the results from the same round, i.e. choose the minimum round instead of the latest round data when both oracles are used.

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Aug 14, 2022
code423n4 added a commit that referenced this issue Aug 14, 2022
@amirnader-ghazvini amirnader-ghazvini added the duplicate This issue or pull request already exists label Aug 29, 2022
@amirnader-ghazvini
Copy link
Collaborator

Duplicate of #179

@amirnader-ghazvini amirnader-ghazvini marked this as a duplicate of #179 Aug 29, 2022
@gititGoro gititGoro added invalid This doesn't seem right out of scope labels Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate This issue or pull request already exists invalid This doesn't seem right out of scope
Projects
None yet
Development

No branches or pull requests

3 participants