Oracle issues

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.sol#L102

Vulnerability details

Impact

Though the vulnerabile part is not directly in scope, but it affects the code in scope. Since due to limited scope i have gathered all the issues in a single report that's why decided to go with high, reasons of which can be seen in next part. Protocol expected behavior may not work as expected by the protocol , since the value returned by oracle might be 0 if the price returned by chainlink oracle is 0 or maybe just due to insufficient validation leading to stale prices.

One of the main impact is the collatRatio calculated by the project can deviate by a certain percentage which might result in user redeem less of his collateral than deserving

Proof of Concept

1.Chainlink has taken oracles offline in some cases. In such a scenario if the oracle price that is fetched in vault.sol throughoracle.latestRoundData(); where it checks this if (block.timestamp > updatedAt + STALE_DATA_TIMEOUT) revert StaleData() and then returns the price like this return answer.toUint256(); https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.sol#L91 But the problem is that if chainlink has taken oracles offline then the answer would be 0 , & the OZ safeCast would pass this value

 function toUint256(int256 value) internal pure returns (uint256) {
        require(value >= 0, "SafeCast: value must be positive");
        return uint256(value);
    }

Thus when a user wants to withdraw or redeem their asset their amount will become 0, due to division by 0, Making withdrawing & redemtion frozen. And if in that time their collateral value goes below the threshold, they can get liquidated easily

2. For stable coins Chainlink might return minAnswer instead of just returning 0

3. In the protocol implementation the roundId is not validated. [Refer to the chainlink docs for this] ,It should not be assumed that updatedAt only returns fresh prices

4. ETH/USD price feed for example have a deviation of 0.5% on mainnet, meaning price will only be updated once the price movement exceeds 0.5% in the heartbeat period. If the market price of WETH is lower than oracle price, it is possible to decrease the amount of collateral and thus possibly leading to arbitrage

Tools Used

Manual Analysis

Recommended Mitigation Steps

1. Add proper safeguard for this case, maybe wrapping them up in a try/catch block

2. Also if the price doesn't go below 0 , if it just crashes like Luna , it's better to have a min/max threshold in check for those events

3. require(answeredInRound >= roundId) the protocol should check for stale or incorrect prices

4.Consider a minting fee higher than the deviation

Assessed type

Oracle

[c4-pre-sort]

JustDravee marked the issue as duplicate of #25

[c4-pre-sort]

JustDravee marked the issue as sufficient quality report

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Out of scope