Uncontrolled usage of Chainlink API for core price retrieval

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L105 https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L459

Vulnerability details

Impact

Chainlink's latestAnswer() usage can yield stale price information, which is crucial for borrowing and liquidation. latestAnswer() is having less ways to be controlled compared to latestRoundData(), which is advised for price sensitive operations.

Staling prices can lead to partial fund losses due via introduction of undercollateralized borrowing and well collateralized liquidation.

Placing severity to medium as the issue is for preventing the translation of Chainlink malfunction to functionality of the system, where the probability of the former is low (but not zero, especially for newer NFT price feeds).

Proof of Concept

FungibleAssetVaultForDAO uses latestAnswer to price collateral:

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/FungibleAssetVaultForDAO.sol#L105

More importantly, NFTVault uses it as a default way to gather current ETH, JPEG and all the market valued NFT prices:

https://github.com/code-423n4/2022-04-jpegd/blob/main/contracts/vaults/NFTVault.sol#L459

Recommended Mitigation Steps

Consider using latestRoundData(), which provides more flexibility and is advised by Chainlink for price retrieval:

https://docs.chain.link/docs/get-the-latest-price/

https://docs.chain.link/docs/historical-price-data/#historical-rounds

https://docs.chain.link/docs/feed-registry/#latestrounddata

An example:

(int256 roundID, int256 priceInUsd, , int256 updatedAt, int256 answeredInRound) = aggregator.latestRoundData();

require(priceInUsd > 0 && updatedAt > 0 && answeredInRound >= roundID , "Price invalid");

[spaghettieth]

Duplicate of #4