search

code-423n4 / 2021-04-maple-findings

0 stars 0 forks source link

Missing check on `setManualPrice(int256 _price)` #85

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

@cmichelio

Vulnerability details

Vulnerability Details

The ChainlinkOracle.setManualPrice function specifies that it can only be called "if manualOverride == true".

This is not the case.

Impact

Assume an oracle failure happened, and the oracle needs to be manually set to prevent losses. The setManualPrice function succeeds and the owner might think that the oracle price is overwritten as the function would fail when manualOverride is not true according to specification. The protocol would still use the broken chainlink price feed and suffer losses.

Recommended Mitigation Steps

Add the missing require(manualOverride == true, "manual override not set") check.

lucas-manuel commented 3 years ago

Not really a bug, but we will address this.

Arachnid commented 3 years ago

Agree with Non-critical as reported.

I would add, though, that the current configuration is unsafe. If setManualPrice is changed to require that manualOverride is true, there will be an interval between calling setManualOverride and setManualPrice during which an old price is used. Instead, a single function that enables the manual override and sets the price should probably be used.

Arachnid commented 3 years ago

On further thought, upgrading to Low based on Warden's reasoning - the success of the call to setManualPrice may lead the submitter to believe that the issue is resolved, when it only sets a value that is not referenced.