DYAD - 0xfox's results

Lines of code

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

Vulnerability details

Impact

The withdraw and redeemDyad functions will revert when attempting to withdraw from any kerosene vault address. This issue will lead to a scenario where users are unable to withdraw their collateral from kerosene vaults, effectively locking their tokens within the protocol.

Proof of Concept

The vulnerable code can be found in the withdraw and redeemDyad functions of VaultManagerV2

Specifically, the lines that cause the issue are: https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L195-L198 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L146-L149

uint value = amount * _vault.assetPrice() * 1e18 / 10**_vault.oracle().decimals() / 10**_vault.asset().decimals();

value is the USD value of the token amount _vault is the passed in vault address cast to Vault oracle is the automatic getter function created for the oracle public variable on Vault

The KerosineVault contract does not have an oracle public variable defined: https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.sol

As a result, when the withdraw function is called with a kerosene vault address, the transaction will revert due to the absence of the oracle function.

Tools Used

Manual review

Recommended Mitigation Steps

Modify the withdraw and redeemDyad functions to handle kerosene vaults differently. The exogenous collateral check should be skipped if the passed in vault address is of a kerosene vault.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

The withdraw function in the VaultManagerV2 contract has a bug that could prevent users from withdrawing their kerosene collateral even when they are sufficiently overcollateralized. This issue could impact a large amount of the user base.

Proof of Concept

This is the context of the withdraw function:

    function withdraw(uint256 id, address vault, uint256 amount, address to) public isDNftOwner(id) {
        if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();
        uint256 dyadMinted = dyad.mintedDyad(address(this), id); //e18
        Vault _vault = Vault(vault);
        uint256 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();
    }

The specific line causing the issue is: https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L150

if (getNonKeroseneValue(id) - value < dyadMinted) revert NotEnoughExoCollat();

id is the id of the user's nft value is the USD value of the token amount to withdraw dyadMinted is the amount of dyad minted by the user

This line checks the non-kerosene collateral value associated with the user's nft minus the withdrawal amount - against the minted dyad of the user, even when the withdrawal is for kerosene collateral. If the non-kerosene collateral value minus the withdrawal amount exceeds the minted dyad - the function will wrongly revert and prevent the user from withdrawing their kerosene.

Tools Used

Manual review

Recommended Mitigation Steps

To mitigate this issue, the withdraw function should be modified to handle endogenous (currently kerosene) and exogenous collateral separately. The function should check if the vault being withdrawn from is a kerosene vault or a non-kerosene vault, and apply the appropriate collateralization check accordingly.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L205-L213

Vulnerability details

Impact

The inability to liquidate a user whose exogenous collateral falls below the value of their minted DYAD can lead to a scenario where under-collateralized positions are not liquidated promptly putting the protocol in financial risk.

Proof of Concept

The liquidate function will only allow the liquidation to proceed if the user's collateralization ratio as calculated by collatRatio function is below the minimum (MIN_COLLATERIZATION_RATIO of 150%). The collatRatio function takes into account both the endogenous (Kerosine) and exogenous collateral when calculating the CR. This means that by holding a sufficiently large ratio of Kerosine tokens a user could prevent themselves from being liquidated even if the value of their exogenous collateral falls below the value of their minted DYAD. Here is the relevant part of the liquidate function:

uint256 cr = collatRatio(id);
if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();

Recommended Mitigation Steps

Modify the liquidate function to allow liquidation based on the value of the exogenous collateral compared to the minted DYAD as well as the overall collateralization ratio.

Tools Used

Manual code review.

Assessed type

Other