hats-finance / Origami-0x998f1b716a5022be026ca6b919c0ddf45ca31abd

GNU Affero General Public License v3.0
2 stars 0 forks source link

`price()`will return the wrong price for asset if the underlying aggregator hits minAnswer #21

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

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

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0x27095c8f3023725b44de9df598d7603ab0c7ff9d0da5d3dd8cc767f458f40979 Severity: medium

Description: Summary:

Chainlink aggregators have a built in circuit breaker if the price of an asset goes outside of a predetermined price band. The result is that if an asset experiences a huge drop in value (i.e. LUNA crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset. This would allow user to continue using the given price with the asset but at the wrong price.

This is exactly what happened to Venus on BSC when LUNA imploded.

Vulnerability Details:

The price() function from chainlink library is extensively used in contracts.

    function price(
        Config memory self,
        uint256 stalenessThreshold,
        OrigamiMath.Rounding roundingMode
    ) internal view returns (uint256) {
        (uint80 roundId, int256 feedValue, , uint256 lastUpdatedAt, uint80 answeredInRound) = self.oracle.latestRoundData();

     . . . some code

    }

price() makes use of Chainlink's latestRoundData() to get the latest price. The self.oracle.latestRoundData() pulls the associated aggregator and requests round data from it. Chainlink aggregators have minPrice and maxPrice circuit breakers built into them. This means that if the price of the asset drops below the minimum price, the protocol will continue to value the token at the minimum price instead of it's actual value. This will allow users to take out huge amounts of bad debt and bankrupt the protocol.

Impact:

If an asset collapses, like the recent USDC depeg, the protocol can be manipulated where the price feeds used due to inflated prices.

Recommendations

Consider using the following minPrice and maxPrice check checks.

For example:

(uint80, int256 answer, uint, uint, uint80) = oracle.latestRoundData();

// minPrice check
require(answer > minPrice, "Min price exceeded");
// maxPrice check
require(answer < maxPrice, "Max price exceeded");
frontier159 commented 8 months ago

We already have price range checks in place for the only place where we consume the latestRoundData(): https://github.com/TempleDAO/origami-public/blob/185a93e25071b6a110ca190e94a6a826e982b2d6/apps/protocol/contracts/common/oracle/OrigamiStableChainlinkOracle.sol#L114-L115