hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

Missing `defaultCurrency`whitelist status check in `OptimisticOracleIntegrator.sol ::_setDefaultCurrencyAndBond`. #114

Open hats-bug-reporter[bot] opened 3 months ago

hats-bug-reporter[bot] commented 3 months ago

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0xc5e976c3f9603471cb4892092bc0106382787b46e2556c26695a46f8535c43e7 Severity: medium

Description: Description\

While setting defaultCurrency in OptimisticOracleIntegrator.sol ::_setDefaultCurrencyAndBond:

   function _setDefaultCurrencyAndBond(address _newCurrency, uint _newBond)
        internal
    {
        if (address(_newCurrency) == address(0)) { //erictee-issue: not checking whitelist status.
            revert Module__OptimisticOracleIntegrator__InvalidDefaultCurrency();
        }
        if (_newBond < oo.getMinimumBond(address(_newCurrency))) {
            revert Module__OptimisticOracleIntegrator__CurrencyBondTooLow();
        }

        defaultCurrency = IERC20(_newCurrency);
        defaultBond = _newBond;
    }

The function is lacking the whitelist status check for defaultCurrency token.

If an unwhitelisted defaultCurrency is configured, assertions will not be able to post properly, resulting in denial of service situation.

Attack Scenario\

When unwhitelisted defaultCurrency is configured, assertions will not be able to post properly, resulting in denial of service situation.

Attachments

NA

  1. Proof of Concept (PoC) File

Refer above.

  1. Revised Code File (Optional)
    • Check the defaultCurrency is an whitelisted currency in OptimisticOracleV3 inside the function OptimisticOracleIntegrator.sol ::_setDefaultCurrencyAndBond first before setting it to the storage.
0xEricTee commented 3 months ago

@PlamenTSV why is this an invalid issue?

FHieser commented 3 months ago

Since we do not have control over possible updates in the Optimistic Oracle regarding whitelists and bonds during the lifetime of the contract, we cannot guarantee that the currency will always be accepted. In case bond or currency become invalid, it is easy to update to a different one. As long as the situation results only in the KPIRewarder not being able to push assertions and no other vulnerabilites, this should be invalid. There is a point to be made that checking for minimum bond and not whitelist is inconsistent and can lead to the transaction reverting with an incorrect error code. That would only count as informational though.

FHieser commented 3 months ago

With this #143 is invalid too