code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

Use of deprecated Chainlink API #173

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

The contract uses Chainlink’s deprecated API latestAnswer(). Such functions might suddenly stop working if Chainlink stopped supporting deprecated APIs.

Impact: Deprecated API stops working. Prices cannot be obtained. Protocol stops and contracts have to be redeployed.

See similar Low-severity finding L11 from OpenZeppelin's Audit of Opyn Gamma Protocol: https://blog.openzeppelin.com/opyn-gamma-protocol-audit/

This was a Medium-severity finding even in the previous version of WildCredit contest as well: code-423n4/2021-07-wildcredit-findings#75 where it was reported that "latestAnswer method will return the last value, but you won’t be able to check if the data is fresh. On the other hand, calling the method latestRoundData allow you to run some extra validations”

Proof of Concept

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

See https://docs.chain.link/docs/deprecated-aggregatorinterface-api-reference/#latestanswer.

Tools Used

Code Review

Recommended Mitigation Steps

Consider to add checks on the return data with proper revert messages if the price is stale or the round is incomplete, for example:

(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = ETH_CHAINLINK.latestRoundData();
require(price > 0, "Chainlink price <= 0");
require(answeredInRound >= roundID, "...");
require(timeStamp != 0, "...");
spaghettieth commented 2 years ago

Duplicate of #4