At ConnextPriceOracle.sol, getPriceFromChainlink() function invokes aggregator.latestRoundData(). However, there are no checks if the return value indicates stale or subzero data. This could lead to wrong prices.
Proof of Concept
function getPriceFromChainlink(address _tokenAddress) public view returns (uint256) {
AggregatorV3Interface aggregator = aggregators[_tokenAddress];
if (address(aggregator) != address(0)) {
(, int256 answer, , , ) = aggregator.latestRoundData();
// It's fine for price to be 0. We have two price feeds.
if (answer == 0) {
return 0;
}
// Extend the decimals to 1e18.
uint256 retVal = uint256(answer);
uint256 price = retVal.mul(10**(18 - uint256(aggregator.decimals())));
return price;
}
ChainLink price data could be stale
Lines of code
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L122-L137
Vulnerability details
Impact
At ConnextPriceOracle.sol,
getPriceFromChainlink()
function invokesaggregator.latestRoundData()
. However, there are no checks if the return value indicates stale or subzero data. This could lead to wrong prices.Proof of Concept
https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol#L122-L137
According to the Chainlink documentation: “A timestamp with zero value means the round is not complete and should not be used.” “if answeredInRound < roundId could indicate stale data.”
Another reference
See example of mitigation here: https://github.com/cryptexfinance/contracts/blob/master/contracts/oracles/ChainlinkOracle.sol#L58-L65
Tools Used
Manual review
Recommended Mitigation Steps
Adding missing the checks to validate the data is not stale