DittoETH
Ditto
DeFiFoundryOracle
55,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

`oracleCircuitBreaker` doesn't check if `chainlinkPrice` and `baseChainlinkPrice` prices are outdated

CipherWizard

Summary

getOraclePrice function can return stale prices when oracle != baseOracle because oracleCircuitBreaker doesn't check if chainlinkPrice and baseChainlinkPrice prices are outdated.

Vulnerability Details

The issue occurs in the oracleCircuitBreaker function below :

function oracleCircuitBreaker(
uint80 roundId,
uint80 baseRoundId,
int256 chainlinkPrice,
int256 baseChainlinkPrice,
uint256 timeStamp,
uint256 baseTimeStamp
) private view {
//@audit not checking: block.timestamp > 2 hours + timeStamp
//@audit not checking: block.timestamp > 2 hours + baseTimeStamp
bool invalidFetchData = roundId == 0 ||
timeStamp == 0 ||
timeStamp > block.timestamp ||
chainlinkPrice <= 0 ||
baseRoundId == 0 ||
baseTimeStamp == 0 ||
baseTimeStamp > block.timestamp ||
baseChainlinkPrice <= 0;
​
if (invalidFetchData) revert Errors.InvalidPrice();
}

As you can see the function verifies many conditions on the input values but it doesn't check if returned prices are outdated or not by verifying that the price timestamp is not too old, this should have been done like in baseOracleCircuitBreaker using : block.timestamp > 2 hours + timeStamp.

Because oracleCircuitBreaker doesn't verify timeStamp and baseTimeStamp, when getOraclePrice is called and oracle != baseOracle either basePrice or price (or both) could be outdated and thus the final the returned price priceInEth could be outdated. This issue can impact all the protocol functions that rely on accurate price feed in their logic.

function getOraclePrice(address asset) internal view returns (uint256) {
AppStorage storage s = appStorage();
AggregatorV3Interface baseOracle = AggregatorV3Interface(s.baseOracle);
uint256 protocolPrice = getPrice(asset);
// prettier-ignore
(
uint80 baseRoundID,
int256 basePrice,
/*uint256 baseStartedAt*/
,
uint256 baseTimeStamp,
/*uint80 baseAnsweredInRound*/
) = baseOracle.latestRoundData();
​
AggregatorV3Interface oracle = AggregatorV3Interface(
s.asset[asset].oracle
);
if (address(oracle) == address(0)) revert Errors.InvalidAsset();
​
if (oracle == baseOracle) {
...
} else {
// prettier-ignore
(
uint80 roundID,
int256 price,
/*uint256 startedAt*/
,
uint256 timeStamp,
/*uint80 answeredInRound*/
) = oracle.latestRoundData();
uint256 priceInEth = uint256(price).div(uint256(basePrice));
oracleCircuitBreaker(
roundID,
baseRoundID,
price,
basePrice,
timeStamp,
baseTimeStamp
);
return priceInEth;
}
}

Impact

The protocol functions that rely on accurate price feed might not work as expected and sometimes can lead to a loss of funds.

Tools Used

Manual review

Recommended Mitigation

Add staleness check in the oracleCircuitBreaker function, it should be modified as follows :

function oracleCircuitBreaker(
uint80 roundId,
uint80 baseRoundId,
int256 chainlinkPrice,
int256 baseChainlinkPrice,
uint256 timeStamp,
uint256 baseTimeStamp
) private view {
bool invalidFetchData = roundId == 0 ||
timeStamp == 0 ||
timeStamp > block.timestamp ||
block.timestamp > 2 hours + timeStamp ||
chainlinkPrice <= 0 ||
baseRoundId == 0 ||
baseTimeStamp == 0 ||
baseTimeStamp > block.timestamp ||
block.timestamp > 2 hours + baseTimeStamp ||
baseChainlinkPrice <= 0;
​
if (invalidFetchData) revert Errors.InvalidPrice();
}
Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Known issues
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-644

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.