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

Missing Validations In Chainlink's latestRoundData Function #144

Closed
code423n4 opened this issue May 2, 2022 · 1 comment
Closed

Missing Validations In Chainlink's latestRoundData Function #144

code423n4 opened this issue May 2, 2022 · 1 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

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol#L74-L80
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/BalancerV2LPOracle.sol#L101-L102
https://github.com/code-423n4/2022-04-mimo/blob/b18670f44d595483df2c0f76d1c57a7bfbfbc083/core/contracts/oracles/GUniLPOracle.sol#L103-L104

Vulnerability details

Impact

Here, latestRoundData() is missing an additional validation to ensure that the round is complete.

Proof of Concept

core/contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol:74:    (, int256 eurAnswer, , uint256 eurUpdatedAt, ) = _eurOracle.latestRoundData();
core/contracts/inception/priceFeed/ChainlinkInceptionPriceFeed.sol:78:    (, int256 answer, , uint256 assetUpdatedAt, ) = _assetOracle.latestRoundData();
core/contracts/oracles/BalancerV2LPOracle.sol:101:    (, int256 answerA, , uint256 assetUpdatedAtA, ) = oracleA.latestRoundData();
core/contracts/oracles/BalancerV2LPOracle.sol:102:    (, int256 answerB, , uint256 assetUpdatedAtB, ) = oracleB.latestRoundData();
core/contracts/oracles/GUniLPOracle.sol:103:    (, int256 answerA, , uint256 assetUpdatedAtA, ) = oracleA.latestRoundData();
core/contracts/oracles/GUniLPOracle.sol:104:    (, int256 answerB, , uint256 assetUpdatedAtB, ) = oracleB.latestRoundData();

Tools Used

Manual code review.
Chainlink best practices.

Recommended Mitigation Steps

Consider adding missing checks.

As an example:

      (uint80 eurRoundID, int256 eurAnswer, , uint256 eurUpdatedAt, uint80 eurAnsweredInRound) = _eurOracle.latestRoundData(); //@audit The eurUpdatedAt data feed property is the timestamp of an answered round and eurAnsweredInRound is the round it was updated in

      require(eurUpdatedAt > 0, "EUR price data is stale"); //@audit A timestamp with zero value means the round is not complete and should not be used.
      require(eurAnswer > 0, "EUR price data not valid");
      require(eurAnsweredInRound >= eurRoundID, "EUR price data is stale"); //@audit If eurAnsweredInRound is less than eurRoundID, the eurAnswer is being carried over. If eurAnsweredInRound is equal to eurRoundID, then the eurAnswer is fresh.
@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 May 2, 2022
code423n4 added a commit that referenced this issue May 2, 2022
@m19 m19 added the duplicate This issue or pull request already exists label May 5, 2022
@m19
Copy link
Collaborator

m19 commented May 5, 2022

Duplicate of #53

@m19 m19 marked this as a duplicate of #53 May 5, 2022
@gzeoneth gzeoneth added the invalid This doesn't seem right label Jun 5, 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
Projects
None yet
Development

No branches or pull requests

4 participants