Summary

In the Compatibilities section of the contest, PriceCalculator is declared as an external dependency (and also is used in production).

This contract has multiple issues which could lead to loss of protocol funds.

Vulnerability Details

There are several issues with PriceCalculator:

• Protocol could use stale Chainlink prices that could lead to borrowing EURS for non-market prices on a falling market.

• DDOS of protocol liquidations in highly fluctuated markets in several networks.

• Incorrect average price calculation on change of Chainlink aggregator in Proxy which could lead to liquidations of collateralised vaults.

Please see the description of each below.

Since it is out of scope, they couldn't be sent as separate issues, but the fact of usage of the vulnerable oracle is the issue of contracts in scope.

Impact

Protocol and users can lose funds in different ways:

• Mint of EURO with non-market prices.

• Delayed liquidations of undercollateralised vaults.

• Liquidations of overcollateralised vaults.

See detailed explanations below.

Tools Used

Manual review

Recommended Mitigation

Use an oracle without vulnerabilities (see how current could be improved below).

Below are findings in the PriceCalculator contract which is formally out-of-scope and provided to explain that the issue above is valid.

[HIGH] Protocol could use stale Chainlink prices that could lead to borrowing EURS for non-market prices on a falling market

Summary

There is no check for stale prices in PriceCalculator (declared dependency).

Vulnerability Details

Chainlink recommends checking that the latestTimestamp of an answer is recent enough:

Your application should track the latestTimestamp variable or use the updatedAt value from the latestRoundData() function to make sure that the latest answer is recent enough for your application to use it. If your application detects that the reported answer is not updated within the heartbeat or within time limits that you determine are acceptable for your application, pause operation or switch to an alternate operation mode while identifying the cause of the delay.

Currently, it is not checked.

Possible scenario:

• The last price for BTC in Chainlink is 30k.

• Falling market, the price becomes 20k.

• Users can mint EUROs for a higher than real price.

• The protocol loses funds due to undercollateralised positions.

Impact

The protocol will lose funds due to undercollateralised positions.

Tools Used

Manual review

Recommended Mitigation

Check that the price is not older than:

• Heartbeat for stable coin collaterals.

• N (for example, 1 hour) for volatile collaterals (BTC, ETH, etc).

[MEDIUM] DDOS of protocol liquidations in highly fluctuated markets in several networks

Summary

There are no restrictions on the while loop cycle count in PriceCalculator::avgPrice, which is called during the call of SmartVaultV3::euroCollateral. This can lead to spending all available gas and reverting liquidation transactions.

Vulnerability Details

Several intro facts that can lead to the DDOS:

• Compatibilities include Any EVM chains with live Chainlink data feeds and live Uniswap pools.

• Some chains could have quite a small block gas limit or even decrease it.

• From Known Issues: Length of accepted tokens array is unchecked, but this TokenManager contract is managed by us and there is unlikely to be more than 5-10 tokens in this array.

• Chainlink facts:

  • Aggregators receive updates from the oracle network only when the Deviation Threshold or Heartbeat Threshold triggers an update during an aggregation round (see https://docs.chain.link/architecture-overview/architecture-decentralized-model?parent=dataFeeds#aggregator).

  • Round intervals could be 30 sec

    Could be checked by running this script on Arbitrum on this block npx hardhat node --fork https://arb1.arbitrum.io/rpc --fork-block-number 166096430

    const hre = require("hardhat");

    ​

    async function main() {

    const aggr = await ethers.getContractAt('AggregatorV3Interface', "0xd0C7101eACbB49F3deCcCc166d238410D6D46d57");

    ​

    let [latestRoundId, , ts, ,] = await aggr.latestRoundData();

    let min = 24n * 3600n;

    for (let i = 0; i < 100; i++) {

    const [round, answ, ts2, ,] = await aggr.getRoundData(latestRoundId - BigInt(i) - 1n);

    if (ts - ts2 < min) {

    min = ts - ts2;

    }

    ts = ts2;

    }

    ​

    console.log("Min rounds interval: ", min);

    }

    ​

    main().catch((error) => {

    console.error(error);

    process.exitCode = 1;

    });

    ​
  • Cost of retrieving price by round id: 7415

    Could be checked in Arbitrum with WBTC aggregator (declared dependency)

    function readFromChainlink() public view {

    uint80 roundId;

    ​

    (roundId, ,,,) = AggregatorV3Interface(0xd0C7101eACbB49F3deCcCc166d238410D6D46d57).latestRoundData();

    ​

    uint startGas;

    for (uint80 i = 0; i<10; i++) {

    startGas = gasleft();

    AggregatorV3Interface(0xd0C7101eACbB49F3deCcCc166d238410D6D46d57).getRoundData(roundId-i-1);

    console.log(roundId-i-1, startGas - gasleft());

    startGas = gasleft();

    }

    }

    ​

• SmartVaultV3::undercollateralised is called twice in SmartVaultManagerV5::liquidateVault:

  • The first time in the check try vault.undercollateralised() returns (bool _undercollateralised) {.

  • The second time inside of vault.liquidate().

Let's suppose the worst case when we have continuous price fluctuations leading to updating asset prices every Chainlink round. The current average period is 4 hours (see PriceCalculator::tokenToEurAvg).

For 1 asset only, for optimal reading prices for these 4 hours, 4 * 60 * 60 / 30 * 7415 = 3_607_200 of gas will be spent.

For 5 accepted tokens, it will cost 3607200 * 5 = 18_036_000, which is more than the proposed Gnosis block gas limit (Since there is a loop by the accepted tokens in SmartVaultV3::euroCollateral).

For 10 tokens: 3607200 * 10 = 36_072_000, which is more than the Ethereum block gas limit.

Yes, it is the worst case, but it is quite realistic. For example for the Arbitrum block 166254591, reading WBTC avg price from `PriceCalculator` costs 711_518. Taking into account the double call of `SmartVaultV3::undercollateralised` and with 10 assets reading cost could be 711_518 * 2 * 10 = 14_230_360, which is not far from block limits

0x6FF84e5bF2Cff6cF1f23071A6f4e2e169D535e97 - is PriceCalculator from the declared dependencies list

function readFromPriceCalculator() public view returns (uint res) {

uint startGas = gasleft();

res = IPriceCalculator(0x6FF84e5bF2Cff6cF1f23071A6f4e2e169D535e97).tokenToEur(

ITokenManager.Token(

0x5742544300000000000000000000000000000000000000000000000000000000,

0x2f2a2543B76A4166549F7aaB2e75Bef0aefC5B0f,

8,

0xd0C7101eACbB49F3deCcCc166d238410D6D46d57,

8

),

1000

);

console.log(startGas - gasleft());

}

​

Impact

On a falling market, delay of liquidations means that the protocol will not be able to sell collateral at a price enough to cover the debt.

Tools Used

Manual review

Recommended Mitigation

PriceCalculator should be improved.

Chainlink uses a VWAP algorithm (https://chain.link/education-hub/twap-vs-vwap) for price calculations and manual avg calculation could be skipped.

[MEDIUM] Incorrect average price calculation on change of Chainlink aggregator in Proxy which could lead to liquidations of collateralised vaults

Summary

• In the PriceCalculator::avgPrice in average price used price outside the 4h interval

• There is no guarantee that there is an answer for round n-1 if there is an answer for n and n > 1.

Vulnerability Details

In the PriceCalculator::avgPrice in average price counts price outside the 4h interval:

• First checked TS of the current round.

• Then retrieved previous round and added value of this round right away, without checking TS.

So average price is always incorrect (since price outside the interval ui used in calculation). It is minor issue, but with the Chainlink issue below it becomes dangerous.

Chainlink uses a proxy for seamless change of aggregator implementations.

Proxy roundId = uint80((uint256(phaseId) << 64) | aggregatorRoundId) where phaseId is the generation of the implementation and aggregatorRoundId is the real round id. So there could be a huge gap between round ids where there is no data. This is not handled in PriceCalculator::avgPrice which is a declared dependency:

​

function avgPrice(uint8 _hours, Chainlink.AggregatorV3Interface _priceFeed) private view returns (uint256) {

uint256 startPeriod = block.timestamp - _hours * 1 hours;

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;

}

So, a possible situation:

• Chainlink deploys a new implementation (for example generation 6)

• Chainlink changes the implementation less than 4h after deployment (for example, on round 3 of the new implementation)

• Right after the change of implementation, avgPrice will try to jump to the past for more than 3 rounds and will retrieve round 6<<64 data with zero answer, which will spoil the average value

See this PoC, where we emulate a request made right after the change of implementation. It should be run on the Ethereum mainnet with hours=4 and pricefeed=0xf4030086522a5beea4988f8ca5b36dbc97bee88c (wbtc/usd)

The result is 1407855500000 (1 btc = 14078 usd) - an incorrect value.
Console output:

I didn't find examples when Chainlink really changed the implementation less than 4h after the deployment of a new aggregator. Because of this, I set the severity to MEDIUM and provided a synthetic example, but there is no guarantee that they will not do it in the future.

Impact

In the worst case, PriceCalculator will return price = realPrice/2 that will allow the liquidation of healthy positions.

Tools Used

Manual review

Recommended Mitigation

PriceCalculator should be improved.

Chainlink uses a VWAP algorithm (https://chain.link/education-hub/twap-vs-vwap) for price calculations and manual avg calculation could be skipped