code-423n4 / 2024-02-wise-lending-findings

11 stars 8 forks source link

Calls to get price from Chainlink may revert #229

Closed c4-bot-9 closed 5 months ago

c4-bot-9 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/OracleHelper.sol#L688 https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseOracleHub/OracleHelper.sol#L714

Vulnerability details

Vulnerability Details

If calls to Chainlink oracles revert this will make all tokens that depend on Chainlink oracles experience a permanent denial of service since the feeds cannot be changed.

OracleHelper.sol#L19C1-L21C10

        if (priceFeed[_tokenAddress] > ZERO_FEED) {
            revert OracleAlreadySet();
        }

Chainlink calls may revert if the Chainlink multisig blocks access to the oracles.

Impact

  1. If calls to Chainlink fail it causes a Denial of Service for tokens that rely directly/indirectly on the Chainlink oracle.

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider using the Solidity try...catch statement to make and handle Chainlink calls, this ensures that the errors can be handled explicitly.

In the catch statement an alternative oracle can be used. The oracle does not need to be available before deployment of the Wiselending protocol. Clearly defining its interface allows the oracle to be added after official deployment.

Assessed type

Access Control

GalloDaSballo commented 5 months ago

It's a good suggestion but could be considered probably QA or OOS due to Readme

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

GalloDaSballo marked the issue as primary issue

c4-judge commented 5 months ago

trust1995 marked the issue as unsatisfactory: Out of scope

nonseodion commented 5 months ago

Hi @trust1995. Thanks for the judging so far. This is not an escalation. I believe this is OOS but I can't find the part of the readme that explicitly says so. I want to find it for future contests. I'd be glad if you can point it out to me. Thanks in advance.

trust1995 commented 5 months ago

The scope section discusses primary / secondary centralization issues are OOS, this would include Chainlink failures.