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

No Return Data Checks on Chainlink's latestRoundData() Might Return Stale Prices #296

Closed
code423n4 opened this issue Aug 17, 2022 · 2 comments
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/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L524
https://github.com/code-423n4/2022-08-frax/blob/c4189a3a98b38c8c962c5ea72f1a322fbc2ae45f/src/contracts/FraxlendPairCore.sol#L532

Vulnerability details

Impact

The _updateExchangeRate function of FraxlendPairCore contract gets price using
chainlink latestRoundData function. However it lacks the check on the return data and
this might lead to stale prices.

Proof of Concept

./FraxlendPairCore.sol:524:            (, int256 _answer, , , ) = AggregatorV3Interface(oracleMultiply).latestRoundData();
./FraxlendPairCore.sol:532:            (, int256 _answer, , , ) = AggregatorV3Interface(oracleDivide).latestRoundData();

Tools Used

Manual Analysis

Recommended Mitigation Steps

I recommend adding checks on the return data as following.

(uint80 roundId, int256 _answer, , uint256 updatedAt, uint80 answeredInRound) = AggregatorV3Interface(oracleMultiply).latestRoundData();
require(answeredInRound >= roundId, "Stale Price");
require(updatedAt != 0, "Round Not Complete");
@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 17, 2022
code423n4 added a commit that referenced this issue Aug 17, 2022
@0xA5DF
Copy link

0xA5DF commented Aug 18, 2022

Out of scope, same as #361

@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

4 participants