c4-bot-8 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L146

Vulnerability details

Impact

The withdraw function relies on chainlink to return the actual price at the time it calls assetPrice function which relies on the latestRoundData function from chainlink oracle contract to fetch the latest price data. However, if the call to latestRoundData reverts due to an issue with the oracle or if the feed is blocked, it could result in a DOS scenario. Since feeds cannot be changed after they are configured, this could lead to a permanent inability to query any prices.

 function withdraw(
        uint id,
        address vault,
        uint amount,
        address to
    ) public isDNftOwner(id) {
        if (idToBlockOfLastDeposit[id] == block.number)
            revert DepositedInSameBlock();
        uint dyadMinted = dyad.mintedDyad(address(this), id);
        Vault _vault = Vault(vault);
        uint value = (amount * _vault.assetPrice() * 1e18) /
            10 ** _vault.oracle().decimals() /
            10 ** _vault.asset().decimals();
        if (getNonKeroseneValue(id) - value < dyadMinted)
            revert NotEnoughExoCollat();
        _vault.withdraw(id, to, amount);
        if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();
    }

 function assetPrice() public view returns (uint) {
        (, int256 answer, , uint256 updatedAt, ) = oracle.latestRoundData();
        if (block.timestamp > updatedAt + STALE_DATA_TIMEOUT)
            revert StaleData();
        return answer.toUint256();
    }

Proof of Concept

A call to latestRoundData could potentially revert, making it impossible to query any prices, hindering the withdrawal process since it wont return the actual price If the feed is blocked or if there is an issue with the oracle, the contract will not be able to fetch the latest price data, resulting in a denial of service scenario. Without proper error handling, the contract will be left in a state where it cannot retrieve price information, affecting any functionality relying on accurate price data.

Tools Used

manual Review

Recommended Mitigation Steps

Surround the call to latestRoundData() with try/catch instead of calling it directly. In a scenario where the call reverts, the catch block can be used to call a fallback oracle or handle the error in any other suitable way.

function assetPrice() public view returns (uint) {
    try oracle.latestRoundData() returns (, int256 answer, , uint256 updatedAt, ) {
        if (block.timestamp > updatedAt + STALE_DATA_TIMEOUT)
            revert StaleData();
        return answer.toUint256();
    } catch {
        // Handle the error in any suitable way, such as calling a fallback oracle.
        revert OracleError();
    }
}

Assessed type

DoS

c4-pre-sort commented 6 months ago

JustDravee marked the issue as duplicate of #25

c4-pre-sort commented 6 months ago

JustDravee marked the issue as sufficient quality report

c4-judge commented 6 months ago

koolexcrypto marked the issue as not a duplicate