c4-bot-2 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/Vault.sol#L91

Vulnerability details

Impact

If there is a problem with chainlink starting a new round and finding consensus on the new value for the oracle (e.g. chainlink nodes abandon the oracle, chain congestion, vulnerability/attacks on the chainlink system) consumers of this contract may continue using outdated stale data (if oracles are unable to submit no new round is started).

This could lead to stale prices and wrong price return value, or outdated price.

As a result, the functions rely on accurate price feed might not work as expected, sometimes can lead to fund loss. The impacts vary and depends on the specific situation like the following:

incorrect liquidation some users could be liquidated when they should not no liquidation is performed when there should be wrong price feed causing inappropriate loan being taken, beyond the current collateral factor

Proof of Concept

No check for round completeness could lead to stale prices and wrong price return value, or outdated price. The functions rely on accurate price feed might not work as expected, sometimes can lead to fund loss.

The oracle wrapper getOraclePrice() call out to an oracle with latestRoundData() to get the price of some token. Although the returned timestamp is checked, there is no check for round completeness.

According to Chainlink's documentation, this function does not error if no answer has been reached but returns 0 or outdated round data. The external Chainlink oracle, which provides index price information to the system, introduces risk inherent to any dependency on third-party data sources. For example, the oracle could fall behind or otherwise fail to be maintained, resulting in outdated data being fed to the index price calculations. Oracle reliance has historically resulted in crippled on-chain systems, and complications that lead to these outcomes can arise from things as simple as network congestion.

Chainlink documentation

Tools Used

Manual Review

Recommended Mitigation Steps


(
            uint80 roundID,
            int signedPrice,
            /*uint startedAt*/,
            uint timeStamp,
            uint80 answeredInRound
        ) = _priceFeed.latestRoundData();
        //check for Chainlink oracle deviancies, force a revert if any are present. Helps prevent a LUNA like issue
        require(signedPrice > 0, "Negative Oracle Price");
        require(timeStamp >= block.timestamp - HEARTBEAT_TIME , "Stale pricefeed");
        require(signedPrice < _maxPrice, "Upper price bound breached");
        require(signedPrice > _minPrice, "Lower price bound breached");
+        require(answeredInRound >= roundID, "round not complete");

        uint256 price = uint256(signedPrice);
        return price;

Assessed type

Oracle

c4-pre-sort commented 2 months ago

JustDravee marked the issue as primary issue

c4-pre-sort commented 2 months ago

JustDravee marked the issue as sufficient quality report

JustDravee commented 2 months ago

Known bug usually caught by bot but I'm amazed that 4naly3er didn't catch it while the detector exists (I personally pushed it): https://github.com/Picodes/4naly3er/blob/master/src/issues/M/staleOracleData.ts Can't say it's OOS if the bot report doesn't have it