search

code-423n4 / 2022-01-notional-findings

1 stars 3 forks source link

Usage of deprecated ChainLink API in `EIP1271Wallet` #197

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

cmichel

Vulnerability details

The Chainlink API (latestAnswer) used in the EIP1271Wallet contract is deprecated:

This API is deprecated. Please see API Reference for the latest Price Feed API. Chainlink Docs

This function does not error if no answer has been reached but returns 0. Besides, the latestAnswer is reported with 18 decimals for crypto quotes but 8 decimals for FX quotes (See Chainlink FAQ for more details). A best practice is to get the decimals from the oracles instead of hard-coding them in the contract.

Recommended Mitigation Steps

Use the latestRoundData function to get the price instead. Add checks on the return data with proper revert messages if the price is stale or the round is uncomplete, for example:

(uint80 roundID, int256 price, , uint256 timeStamp, uint80 answeredInRound) = priceOracle.latestRoundData();
require(answeredInRound >= roundID, "...");
require(timeStamp != 0, "...");
jeffywu commented 2 years ago

Duplicate #178

pauliax commented 2 years ago

Valid finding. I am hesitating whether this should be low or medium but decided to leave it as a medium because the likeliness is low but the impact would be huge, and all the wardens submitted this with a medium severity. Also: "Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements."

pauliax commented 2 years ago

Making this a primary issue, because it contains the most concise description.