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

1 stars 1 forks source link

Use of deprecated oracle API in `_normalizeAggregatorAnswer` #229

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Use of deprecated oracle API in _normalizeAggregatorAnswer

Likelihood low, impact high.

The Chainlink latestAnswer function included in IAggregatorV3Interface and called in NFTVault#_normalizeAggregatorAnswer() is considered deprecated and no longer included in the Chainlink API documentation.

It's considered best practice to use the latestRoundData function instead. (API docs). latestAnswer returns only the value of the latest price, whereas latestRoundData returns additional information that can be used to validate whether a price is stale:

NFTVault#459:

        int256 answer = oracle.latestAnswer();
        uint8 decimals = oracle.decimals();

Example using latestRoundData:

        (uint80 roundId,
        int256 answer,
        uint256 startedAt,
        uint256 updatedAt,
        uint80 answeredInRound) = oracle.latestRoundData();
        require(answeredInRound >= roundId, "Stale data");
        require(timestamp != 0, "Zero timestamp");
        uint8 decimals = oracle.decimals();

Stale prices may impact collateral value and credit limit calculations, incorrectly reporting a position as under- or overcollateralized.

See also OpenZeppelin's guidelines for safely using latestRoundData and latestAnswer, and consider the impact of a stale or reverting price feed.

spaghettieth commented 2 years ago

Duplicate of #4