The Standard
The Standard
DeFiHardhat
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

SmartVaultV3 : function `euroCollateral` is susceptible to be impacted by incorrect price value.

ak1

Summary

The function euroCollateral() use's the function tokenToEurAvg which is using chain-link's pricing function to get the price of an asset. Here, the number of hours used in averaging is not sufficient.

Vulnerability Details

The function euroCollateral() would be used to compute the amount of EUORs to be minted for the available asset balance.
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L67-L73

function euroCollateral() private view returns (uint256 euros) {
ITokenManager.Token[] memory acceptedTokens = getTokenManager().getAcceptedTokens();
for (uint256 i = 0; i < acceptedTokens.length; i++) {
ITokenManager.Token memory token = acceptedTokens[i];
euros += calculator.tokenToEurAvg(token, getAssetBalance(token.symbol, token.addr)); ---------->> refer this call.
}
}

Later the calculated euros would be used in many places. one of the place is maxMintable.
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L75-L77

function maxMintable() private view returns (uint256) {
return euroCollateral() * ISmartVaultManagerV3(manager).HUNDRED_PC() / ISmartVaultManagerV3(manager).collateralRate();
}

the output of maxMintable() would be used to decide the liquidation condition by using the below function.
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L99-L101

function undercollateralised() public view returns (bool) {
return minted > maxMintable();
}

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L114-L123

function liquidate() external onlyVaultManager {
require(undercollateralised(), "err-not-liquidatable"); -------->> refer here.
liquidated = true;
minted = 0;
liquidateNative();
ITokenManager.Token[] memory tokens = getTokenManager().getAcceptedTokens();
for (uint256 i = 0; i < tokens.length; i++) {
if (tokens[i].symbol != NATIVE) liquidateERC20(IERC20(tokens[i].addr));
}
}

But, when we look at the function tokenToEurAvg,
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/utils/PriceCalculator.sol#L43-L49

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); --------------------------->> avg of 4 hours is not sufficient
(, int256 eurUsdPrice,,,) = clEurUsd.latestRoundData(); ------------------------>> lack of freshness check like last created time, valid value and so.
return collateralUsd / uint256(eurUsdPrice);
}

It is using the chain-link price data to calculate the asset price. Since this place misses the validation of stale price, the old price value would influence the asset price calculation. This would directly affect on the function which are in scope.

also, the hours used in averaging is not sufficient.
https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/utils/PriceCalculator.sol#L18-L37

function avgPrice(uint8 _hours, Chainlink.AggregatorV3Interface _priceFeed) private view returns (uint256) {
uint256 startPeriod = block.timestamp - _hours * 1 hours; ----------->> _hours is passed as 4 this is not sufficient
uint256 roundTS;
uint80 roundId;
int256 answer;
(roundId, answer,, roundTS,) = _priceFeed.latestRoundData();
uint256 accummulatedRoundPrices = uint256(answer);
uint256 roundCount = 1;
while (roundTS > startPeriod && roundId > 1) {
roundId--;
try _priceFeed.getRoundData(roundId) {
(, answer,, roundTS,) = _priceFeed.getRoundData(roundId);
accummulatedRoundPrices += uint256(answer);
roundCount++;
} catch {
continue;
}
}
return accummulatedRoundPrices / roundCount;
}

Impact

stale price would be causing impact on the liquidation. Some time it would be lead to early liquidation or missing from liquidation.

Tools Used

Manual review.

Recommendations

We suggest to follow the chain-links recommended procedure when calculating the asset price.

Please update the function tokenToEurAvg to fix this issue.

https://docs.chain.link/data-feeds/using-data-feeds

Use number of hours as 24 in averaging.

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Chainlink-price

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Chainlink-price

Support

FAQs

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