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

Liquidation may fail if PriceFeed.aggregator() is updated

tripathi

Summary

Liquidation may fail if PriceFeed.aggregator() is updated while Liquidataion pool uses hardcoded eurUsd pricefeed from the deployment

Vulnerability Details

File: contracts/LiquidationPoolManager.sol
constructor(address _TST, address _EUROs, address _smartVaultManager, address _eurUsd, address payable _protocol, uint32 _poolFeePercentage) {
pool = address(new LiquidationPool(_TST, _EUROs, _eurUsd, ISmartVaultManager(_smartVaultManager).tokenManager()));
TST = _TST;
EUROs = _EUROs;
smartVaultManager = _smartVaultManager;
protocol = _protocol;
poolFeePercentage = _poolFeePercentage;
}

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPoolManager.sol#L23

LiquidationPoolManager deploys LiquidationPool and set the priceFeed _eurUSD in a immuatable which cant changed in the future

File: contracts/LiquidationPool.sol
​
address private immutable TST;
address private immutable EUROs;
address private immutable eurUsd;
​
//................
​
​
constructor(
address _TST,
address _EUROs,
address _eurUsd,
address _tokenManager
) {
TST = _TST; //governance token
EUROs = _EUROs;
eurUsd = _eurUsd;
tokenManager = _tokenManager;
manager = payable(msg.sender); //deployer
}

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L18

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L34
while in the priceFeed contract, the address of aggregator can be changed by owner

function confirmAggregator(address _aggregator)
external
onlyOwner()
{
require(_aggregator == address(proposedAggregator), "Invalid proposed aggregator");
delete proposedAggregator;
setAggregator(_aggregator);
}
​
​
/*
* Internal
*/
​
function setAggregator(address _aggregator)
internal
{
uint16 id = currentPhase.id + 1;
currentPhase = Phase(id, AggregatorV2V3Interface(_aggregator));
phaseAggregators[id] = AggregatorV2V3Interface(_aggregator);
}
...
function aggregator()
external
view
returns (address)
{
return address(currentPhase.aggregator);
}
​

https://etherscan.io/address/0xc7de7f4d4C9c991fF62a07D18b3E31e349833A18#code

Also chainLink documents says : you can call functions on the aggregator directly, but it is a best practice to use the AggregatorV3Interface to run functions on the proxy instead so that changes to the aggregator do not affect your application. Read the aggregator contract only if you need functions that are not available in the proxy.
https://docs.chain.link/data-feeds/price-feeds/api-reference#aggregatorv3interface

As this function is called internally during liquidation of smartVaults ,may prevent liquidation due to unavaibility/stale price of previous eurUSD priceFeed

Tools Used

Manual

Recommendations

considering current implementation adding a setter with admin acess control could prevent this issue

Updates

Lead Judging Commences

hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

tripathi Submitter
over 1 year ago
hrishibhat Lead Judge
over 1 year ago
hrishibhat Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

informational/invalid

Support

FAQs

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