Summary
The Chainlink's AggregatorV3Interface(_priceFeed).latestRound() returns various variables, among which only price is received, which may lead to stale price and incomplete rounds
Vulnerability Details
The Chainlink's AggregatorV3Interface(_priceFeed).latestRound() returns the following variables:
roundId
answer (price)
startedAt
updatedAt
answeredInRound
These return values are required for some extra checks before updating the price . By just receiving the price & not checking other values, you can get stale prices & incomplete rounds.
In LiquidationPool::distributeAssets() as we can seen below this has been done at two places while receiving the data of price of assets and eurusd.
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable {
consolidatePendingStakes();
@> (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
for (uint256 j = 0; j < holders.length; j++) {
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0) {
for (uint256 i = 0; i < _assets.length; i++) {
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0) {
@> (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
uint256 _portion = asset.amount * _positionStake / stakeTotal;
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd)
* _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs) {
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0)) {
nativePurchased += _portion;
} else {
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}
And also in PriceCalculator.sol (even if this was not in scope still important), functions :- tokenToEurAvg, tokenToEur, eurToToken also contain same things
function tokenToEurAvg(ITokenManager.Token memory _token, uint256 _tokenValue) external view returns (uint256) {
Chainlink.AggregatorV3Interface tokenUsdClFeed = Chainlink.AggregatorV3Interface(_token.clAddr);
uint256 scaledCollateral = _tokenValue * 10 ** getTokenScaleDiff(_token.symbol, _token.addr);
uint256 collateralUsd = scaledCollateral * avgPrice(4, tokenUsdClFeed);
@> (, int256 eurUsdPrice,,,) = clEurUsd.latestRoundData();
return collateralUsd / uint256(eurUsdPrice);
}
function tokenToEur(ITokenManager.Token memory _token, uint256 _tokenValue) external view returns (uint256) {
Chainlink.AggregatorV3Interface tokenUsdClFeed = Chainlink.AggregatorV3Interface(_token.clAddr);
uint256 scaledCollateral = _tokenValue * 10 ** getTokenScaleDiff(_token.symbol, _token.addr);
@> (,int256 _tokenUsdPrice,,,) = tokenUsdClFeed.latestRoundData();
uint256 collateralUsd = scaledCollateral * uint256(_tokenUsdPrice);
@> (, int256 eurUsdPrice,,,) = clEurUsd.latestRoundData();
return collateralUsd / uint256(eurUsdPrice);
}
function eurToToken(ITokenManager.Token memory _token, uint256 _eurValue) external view returns (uint256)
{
Chainlink.AggregatorV3Interface tokenUsdClFeed = Chainlink.AggregatorV3Interface(_token.clAddr);
@> (, int256 tokenUsdPrice,,,) = tokenUsdClFeed.latestRoundData();
@> (, int256 eurUsdPrice,,,) = clEurUsd.latestRoundData();
return _eurValue * uint256(eurUsdPrice) / uint256(tokenUsdPrice) / 10 ** getTokenScaleDiff(_token.symbol, _token.addr);
}
Impact
Since Chainlink's oracle for receiving data about price has been used at various places , the system depends a lot on it.
If there is a problem with Chainlink starting a new round and finding consensus on the new value for the oracle (e.g. Chainlink nodes abandon the oracle, chain congestion, vulnerability/attacks on the chainlink system) consumers may continue using outdated stale or incorrect data (if oracles are unable to submit no new round is started).
Tools Used
Thorough manual reviewing of codebase
Recommendations
Its recommended adding checks to ensure the data retrieved from the Chainlink oracle is valid and up-to-date.
To avoid potential issues like getting stale prices or incomplete rounds, it's better to also use the other return values (roundId, startedAt, updatedAt, answeredInRound) to validate the data.
Like for example in LiquidationPool::distributeAssets() while receiving price data of assets, following code can be used :
function distributeAssets(ILiquidationPoolManager.Asset[] memory _assets, uint256 _collateralRate, uint256 _hundredPC) external payable
{
consolidatePendingStakes();
(,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
uint256 stakeTotal = getStakeTotal();
uint256 burnEuros;
uint256 nativePurchased;
for (uint256 j = 0; j < holders.length; j++)
{
Position memory _position = positions[holders[j]];
uint256 _positionStake = stake(_position);
if (_positionStake > 0)
{
for (uint256 i = 0; i < _assets.length; i++)
{
ILiquidationPoolManager.Asset memory asset = _assets[i];
if (asset.amount > 0)
{
- (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
+ (uint80 roundID,
+ int256 assetPriceUsd,
+ uint256 startedAt,
+ uint256 updatedAt,
+ uint80 answeredInRound) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
+ require(assetPriceUsd > 0, "Asset price must be greater than 0");
+ require(answeredInRound >= roundID, "Stale price data");
+ require(updatedAt != 0, "Incomplete round");
uint256 _portion = asset.amount * _positionStake / stakeTotal;
uint256 costInEuros = _portion * 10 ** (18 - asset.token.dec) * uint256(assetPriceUsd) / uint256(priceEurUsd) * _hundredPC / _collateralRate;
if (costInEuros > _position.EUROs)
{
_portion = _portion * _position.EUROs / costInEuros;
costInEuros = _position.EUROs;
}
_position.EUROs -= costInEuros;
rewards[abi.encodePacked(_position.holder, asset.token.symbol)] += _portion;
burnEuros += costInEuros;
if (asset.token.addr == address(0))
{
nativePurchased += _portion;
}
else
{
IERC20(asset.token.addr).safeTransferFrom(manager, address(this), _portion);
}
}
}
}
positions[holders[j]] = _position;
}
if (burnEuros > 0) IEUROs(EUROs).burn(address(this), burnEuros);
returnUnpurchasedNative(_assets, nativePurchased);
}
Similarly , changes can be made at other places too
