Bauchibred - Pricing on liquidations can still be bogus

[sherlock-admin]

Bauchibred

medium

Pricing on liquidations can still be bogus

Summary

This was originally submitted by hyh in the V1 contest, found here, issue was submitted, the mitigation applied doesn't still does not solve this as the check was applied to the price immediately returned from latestRoundData but Hubble deals in 6 decimals and if answer < 100
the division at L35
answer /= 100; sidesteps the non-positive check at L34, i.e in this scenario the submitted issue is not really mitigated against

NB: A similar case can also be made for formatPrice(), and the getUnderLyingPrice function is extensively used in: AMM.sol, orderbook.sol, IF.sol... all within Hubble.

Vulnerability Detail

See summary, and then take a look at Oracle.sol#L24-L36

    function getUnderlyingPrice(address underlying)
        virtual
        external
        view
        returns(int256 answer)
    {
        if (stablePrice[underlying] != 0) {
            return stablePrice[underlying];
        }
        (,answer,,,) = AggregatorV3Interface(chainLinkAggregatorMap[underlying]).latestRoundData();
        require(answer > 0, "Oracle.getUnderlyingPrice.non_positive");
                //@audit anything below 100 would result in the value returning 0 as the price, so the protection against a zero price does not really protect
        answer /= 100;
    }

Check the @audit tag, whereas one could argue that for this to happen price has already gone very low and in the case of the index/mark ideology it doesn't count since index price < 100 (in 8 decimals) will probably mean 0 in 6 decimals. All other instances of implementing getUnderlyingPrice() would be affected, be it the liquidations or implementations in InsuranceFund.sol or clearingHouse.sol...

Note that this can easily allow a liquidation at a non-market price that happen to not even be printed in the Oracle feed, for more insights, since other erc20 are going to be added to, A flash crash could happen to any of the future erc20 to be integrated, it easily means that users could easily be liquidated at a value, where they shouldn't be liquidatable.

Hypothetical POC

• User provides a good amount of collateral
• Borrows a very little amount to ensure that he almost never gets liquidated
• A flash price crash happens
• User could easily get liquidated when his collateral is at an edge case 2-99x more than the needed healthy level
• To even worsen the issue, If the prices go back up, user has already gotten liquidated and can't get his funds back.
• Key to note that it's not uncommon that during a flash crash, the prices can outrageously drop, go back up to be stable for a while
• A user who is waiting for this scenario since he believes he has 90x the health level would unfairly get liquidated. muser might want to take back his funds at this time but he already got liquidated.

Impact

Same as previous report, but even worse since in this case attacker doesn't even need to wait for a price outbreak but rather when it's just less than 100

Code Snippet

Oracle.sol#L24-L36

Tool used

• Manual Audit
• Code4rena hubble findings/46

Recommendation

This all stems from incorrectly checking for non-positive prices, so this should be fixed, i.e amount should be checked to be above it's denominator

Duplicate of #99