search

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

1 stars 1 forks source link

Use of deprecated oracle API in `_collateralPriceUsd` #231

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/FungibleAssetVaultForDAO.sol#L105

Vulnerability details

Likelihood low, impact high.

The Chainlink latestAnswer function included in IAggregatorV3Interface and called in FungibleAssetVaultForDAO#_collateralPriceUsd() 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:

[FungibleAssetVaultForDAO#105]((https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/vaults/FungibleAssetVaultForDAO.sol#L105)

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

Example:

        (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 used in the borrow and withdraw functions, incorrectly reporting the vault as over- or undercollateralized.

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