DYAD - iamandreiski's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L64-L65 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L95 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L250-L267 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L269-L286

Vulnerability details

Impact

In the deploy script weth and wsteth vaults are added to the Kerosine Manager, as well as the unbounded Kerosine vault is added to the Vault Manager Licenser, which allows for a malicious users to add the weth and wsteth vault as part of the vaultsKerosene mapping and add the unbounded kerosene vault to the vaults mapping. This would double the amount of collateral that they have in each vault allowing them to mint undercollateralized DYAD and exploit the system.

Proof of Concept

In the deploy script, the weth and wsteth vaults are added and licensed to the Kerosine Manager:

 kerosineManager.add(address(ethVault));
 kerosineManager.add(address(wstEth));

As well as being added to the vault licenser:

 vaultLicenser.add(address(ethVault));
    vaultLicenser.add(address(wstEth));
    vaultLicenser.add(address(unboundedKerosineVault));

Exploit PoC:

• A malicious user notices that the ethVault and the wstEth vault are part of both the Kerosine Manager (licensed) and are also licensed as part of the vault licenser.
• They add the ethVault to both the regular vaults mapping and the kerosine vaults mapping:

function add(uint256 id, address vault) external isDNftOwner(id) {
        if (vaults[id].length() >= MAX_VAULTS) revert TooManyVaults();
        if (!vaultLicenser.isLicensed(vault)) revert VaultNotLicensed();
        if (!vaults[id].add(vault)) revert VaultAlreadyAdded();
        emit Added(id, vault);
    }

Kerosene:

 function addKerosene(uint256 id, address vault) external isDNftOwner(id) {
        if (vaultsKerosene[id].length() >= MAX_VAULTS_KEROSENE) revert TooManyVaults();
        if (!keroseneManager.isLicensed(vault)) revert VaultNotLicensed();
        if (!vaultsKerosene[id].add(vault)) revert VaultAlreadyAdded();
        emit Added(id, vault);
    }

Since the vaults are licensed both in the vault licenser and in the Kerosene Manager, the keroseneManager.isLicensed checks the Kerosene Manager vaults set to see if the vault is contained there:

function isLicensed(
    address vault
  ) 
    external 
    view 
    returns (bool) {
      return vaults.contains(vault);
  }

And since the vault was added using the deploy script, this check will pass adding the vault to the vaultsKerosene in the Vault Manager as well.

• The malicious user adds collateral to the weth and wsteth vault, let's say that they've added $1,000 worth of collateral in each vault.
• Instead of the system calculating the totalUsdValue at 2,000 USD, it will be 4,000 USD, basically doubling their ability to mint undercollateralized DYAD and exploit the system.
• This is because the nonKeroseneValue will search the vaults mapping to calculate the amount of available collateral:

 function getNonKeroseneValue(uint256 id) public view returns (uint256) {
        uint256 totalUsdValue;
        uint256 numberOfVaults = vaults[id].length();
        for (uint256 i = 0; i < numberOfVaults; i++) {
            Vault vault = Vault(vaults[id].at(i));
            uint256 usdValue;
            if (vaultLicenser.isLicensed(address(vault))) {
                usdValue = vault.getUsdValue(id);
            }
            totalUsdValue += usdValue;
        }
        return totalUsdValue;
    }

And the KeroseneValue will search the vaultsKerosene mapping to also calculate the amount of available collateral:

function getKeroseneValue(uint256 id) public view returns (uint256) {
        uint256 totalUsdValue;
        uint256 numberOfVaults = vaultsKerosene[id].length();
        for (uint256 i = 0; i < numberOfVaults; i++) {
            Vault vault = Vault(vaultsKerosene[id].at(i));
            uint256 usdValue;
            if (keroseneManager.isLicensed(address(vault))) {
                usdValue = vault.getUsdValue(id);
            }
            totalUsdValue += usdValue;
        }
        return totalUsdValue;
    }

• This will effectively double the collateral that the malicious user inserted into the system, allowing them to mint DYAD worth ~2650 USD instead of ~1325 USD, with only 2000 USD worth of real collateral, effectively gaining 650 USD, gaming the system and minting undercollateralized DYAD.

The above example has demonstrated an exploit with a low amount of collateral, as there is no upper limit how much collateral one can deposit or a limit of how much DYAD can be minted, this can be an exploit of a much greater scale.

Tools Used

Manual Review

Recommended Mitigation Steps

Include the weth/wsteth vaults only in the vault licenser, and the kerosine vaults only in the kerosine manager.

Assessed type

Invalid Validation

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/staking/KerosineDenominator.sol#L21

Vulnerability details

Impact

The way that the Kerosene Denominator is currently calculated, it takes the total supply of Kerosene and subtracts the Kerosene in the protocol owner's multisig. The problem with this is that it never accounts for the bounded supply being considered 2x, thus inflating the overall Kerosene price. This effect will be larger, the more Kerosene gets locked up in the bounded vault(s).

Proof of Concept

Bounded Kerosene tokens are counted as part of the Kerosene's total supply. So in order to calculate the denominator or the tokens which are in circulation, the team uses the total supply and subtracts the protocol owner's tokens.

return kerosine.totalSupply() - kerosine.balanceOf(MAINNET_OWNER);

The problem is that it doesn't account for the Kerosene tokens whose value is double effectively inflating the overall Kerosene token price.

The more tokens get locked up, Kerosene's price will get inflated even more which means that DYAD's collateralization level will be lower.

Tools Used

Manual Review

Recommended Mitigation Steps

Include the amount of tokens currently present in the bounded vault to the total supply and then subtract the balance in the owner's account.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

The asset price of unbounded kerosene is wrongly calculated by getting the vaults from the Kerosene manager, which will also include the Kerosene vaults, thus accounting for the Kerosene amount + value in the total calculation as well. All Kerosene vaults would have to be part of the Kerosene manager in order for them to be licensed and added to the Vault Manager. By adding them to the Kerosene Manager we will be calculating their supply as well when calculating the price of Kerosene.

Proof of Concept

This is also connected to another vulnerability which relates to adding/licensing weth and wsteth vaults to the Kerosene Manager as well, besides the vault licenser.

Weth and wsteth vaults should be added to the Vault Licenser only, while the Kerosene vaults to the Kerosene Manager - only.

The second part of the vulnerability is that currently the assetPrice function within the unbounded vault is getting the vault list from the Kerosene Manager, instead of the vault licenser.

Since the vault licenser should contain the exogenous collateral vaults only and calculating the Kerosene value based on the exogenous vaults only, should be the way in which it's done.

When the assetPrice of the unbounded vault is calculated:

   function assetPrice() public view override returns (uint256) {
        uint256 tvl;
        address[] memory vaults = kerosineManager.getVaults();
        uint256 numberOfVaults = vaults.length;
        for (uint256 i = 0; i < numberOfVaults; i++) {
            Vault vault = Vault(vaults[i]);
            tvl += vault.asset().balanceOf(address(vault)) * vault.assetPrice() * 1e18
                / (10 ** vault.asset().decimals()) / (10 ** vault.oracle().decimals());
        }
        uint256 numerator = tvl - dyad.totalSupply();
        uint256 denominator = kerosineDenominator.denominator();
        return numerator * 1e8 / denominator;
    }

It can be seen that the vaults are being taken from the Kerosene Manager and subsequently the amount/value based on that.

In order for the Kerosene vaults to work properly, they would need to be added/licensed to the Kerosene Manager, due to this requirement in the Vault Manager:

 function addKerosene(uint256 id, address vault) external isDNftOwner(id) {
        if (vaultsKerosene[id].length() >= MAX_VAULTS_KEROSENE) revert TooManyVaults();
        if (!keroseneManager.isLicensed(vault)) revert VaultNotLicensed();
        if (!vaultsKerosene[id].add(vault)) revert VaultAlreadyAdded();
        emit Added(id, vault);
    }

Adding Kerosene vaults to the Kerosene Manager will also include them in the calculation of Kerosene price as well.

Taking all of the above into consideration, in order for the Kerosene vaults to work properly, they need to be added to the Kerosene Manager only, and in order for exogenous collateral vaults to work properly they need to be added to the Vault Licenser - only.

Tools Used

Manual Review

Recommended Mitigation Steps

Use the vault licenser to fetch vaults in the calculation of the Kerosene price, add only exogenous collateral vaults to the Vault licenser. The Kerosene Manager should contain Kerosine vaults only.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

Withdrawing Kerosene will never be possible since VaultManagerV2 is not compliant with how Kerosene vaults operate. The exogenous collateral vaults have an oracle through which they fetch their prices, but Kerosene vaults don't use oracles and their price is based upon the supply of exogenous collateral. When withdrawing or redeeming collateral/assets, the withdraw / redeem function calls vault.oracle.decimals(), but since there's no oracle in the Kerosene vaults, this will always revert.

Proof of Concept

After depositing Kerosene to a vault, if a user wants to withdraw it, through either the withdraw() function or redeem their DYAD for Kerosene through the redeem() function, it will always revert:

 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);
        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();
    }

This is because the function wants to calculate the value by calling the _vault.oracle().decimals():

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

But since there's no oracle() within the Kerosene vaults:

 IVaultManager   public immutable vaultManager;
  ERC20           public immutable asset;
  KerosineManager public immutable kerosineManager;

This will always revert, thus rendering the withdraw function/mechanism unusable forever.

We can see the same "mechanism" present in the redeem() function as well:

 function redeemDyad(uint256 id, address vault, uint256 amount, address to)
        external
        isDNftOwner(id)
        returns (uint256)
    {
        dyad.burn(id, msg.sender, amount);
        Vault _vault = Vault(vault);
        uint256 asset =
            amount * (10 ** (_vault.oracle().decimals() + _vault.asset().decimals())) / _vault.assetPrice() / 1e18;
        withdraw(id, vault, asset, to);
        emit RedeemDyad(id, vault, amount, to);
        return asset;
    }

In order for the Kerosene vault withdraw() function to be called, it has to be through the VaultManager, otherwise it will revert due to the onlyVaultManager modifier:

  function withdraw(uint256 id, address to, uint256 amount) external onlyVaultManager {
        id2asset[id] -= amount;
        asset.safeTransfer(to, amount);
        emit Withdraw(id, to, amount);
    }

So the only way to withdraw funds is through the VaultManager, and that functionality is unusable.

Tools Used

Manual Review

Recommended Mitigation Steps

Integrate separate withdraw/redeem functions for Kerosene and/or within the withdraw and redeem function check if the vault in-question is Kerosene, and if it is - use a different flow.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L65 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L165

Vulnerability details

Impact

When someone mints new DYAD, the only requirement for exogenous collateral is for its value to be greater or equal than the amount of DYAD minted, which would mean at least 100%, the general collateralization requirement only checks if the total collateral (exogenous + kerosine) is greater than 150%. Meaning that 50% of the collateral value can be kerosine.

If the value of exogenous collateral falls below 100%, this will render the system unusable until its again above 100% due to the Kerosine asset price calculation reverting due to overflow/underflow.

Proof of Concept

When someone wants to mean new DYAD, the collateralization requirement is for the exogenous collateral value to be equal or greater than the amount of DYAD minted:

 uint256 newDyadMinted = dyad.mintedDyad(address(this), id) + amount;
 if (getNonKeroseneValue(id) < newDyadMinted) revert NotEnoughExoCollat();

Subsequently the total collateral check (at least 150% of the value of DYAD) checks for the non-Kerosene value + Kerosene value:

if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();

This means that 50% of the collateral value of DYAD can be Kerosene. The price of Kerosene is calculated via the following function:

   uint tvl;
      address[] memory vaults = kerosineManager.getVaults();
      uint numberOfVaults = vaults.length;
      for (uint i = 0; i < numberOfVaults; i++) {
        Vault vault = Vault(vaults[i]);
        tvl += vault.asset().balanceOf(address(vault))
                * vault.assetPrice() * 1e18
                / (10**vault.asset().decimals())
                / (10**vault.oracle().decimals());
      }

    uint numerator   = tvl - dyad.totalSupply();
      uint denominator = kerosineDenominator.denominator();
      return numerator * 1e8 / denominator;

This means that the price of Kerosene is dependent on the value of the collateral in the DYAD vaults.

Due to the only hard requirement for the exogenous collateral being for it to be equal or greater than 100%, this means that it can easily fall below it, especially during bear markets / black swan events / or bank-run style crisis (SVB crisis of March 2023).

If the amount of exogenous collateral falls below 100%, this line will always panic revert due to underflow/overflow:

uint numerator = tvl - dyad.totalSupply();

as the tvl will be less than the total supply of DYAD.

Tools Used

Manual Review

Recommended Mitigation Steps

Increase the hard requirement for exogenous collateral to be at least 120% for a greater buffer.

Assessed type

Invalid Validation

Lines of code

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

Vulnerability details

Impact

In the DYAD protocol Kerosene is used as part of the collateral. It can account for up to 50% of the collateral, although during liquidations, even though Kerosene is accounted for when the collateral ratio is calculated, it's never transferred to the liquidator from the Kerosene vaults, which can cause a multitude of negative effects including the liquidator receiving less collateral than needed and the liquidated subject getting liquidated for less collateral than it's supposed to.

Proof of Concept

When users deposit collateral and subsequently mint DYAD, the first requirement is that their exogenous collateral is at least a 100% of the amount minted, which allows for Kerosene to account for almost 50% of the collateral:

 function mintDyad(uint256 id, uint256 amount, address to) external isDNftOwner(id) {
        uint256 newDyadMinted = dyad.mintedDyad(address(this), id) + amount;
        if (getNonKeroseneValue(id) < newDyadMinted) revert NotEnoughExoCollat();
        dyad.mint(id, to, amount);
        if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();

And here is how the collateral ratio is calculated:

 function collatRatio(uint256 id) public view returns (uint256) {
        uint256 _dyad = dyad.mintedDyad(address(this), id);
        if (_dyad == 0) return type(uint256).max;
        return getTotalUsdValue(id).divWadDown(_dyad);
    }

Subsequently the getTotalUsdValue():

    function getTotalUsdValue(uint256 id) public view returns (uint256) {
        return getNonKeroseneValue(id) + getKeroseneValue(id);
    }

The problem is when users need to be liquidated, the Kerosene amount is accounted for in the liquidation ratio/reward calculation but it's never moved/transferred to the liquidator.

When a subject's position becomes undercollateralized with their collateral ratio reaching 1.5e18 or below, they will be eligible for liquidation. In the liquidate() function, the collateral ratio is first checked, which includes both Kerosene and Non-kerosene values:

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

After which, the liquidation asset share is calculated:

  uint256 cappedCr = cr < 1e18 ? 1e18 : cr;
        uint256 liquidationEquityShare = (cappedCr - 1e18).mulWadDown(LIQUIDATION_REWARD);
        uint256 liquidationAssetShare = (liquidationEquityShare + 1e18).divWadDown(cappedCr);

Even though the Kerosene was included in the calculation of the liquidation reward and the collateral that would need to be transferred, only the regular vaults are accounted for as the keroseneVaults mapping is never iterated over:

uint256 numberOfVaults = vaults[id].length();
        for (uint256 i = 0; i < numberOfVaults; i++) {
            Vault vault = Vault(vaults[id].at(i));
            uint256 collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);
            vault.move(id, to, collateral);
        }

This will result in transferring less collateral than it's needed to the liquidator while they burned the full DYAD amount which is a loss of funds.

Tools Used

Manual Review

Recommended Mitigation Steps

Include a mechanism in which while liquidating the equivalent Kerosene amounts will be transferred as well.

Assessed type

Token-Transfer

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L56-L64 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L205-L228 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L165

Vulnerability details

Impact

There are multiple factors that can lead to the under-collateralization of DYAD and its subsequent depegging.

• The exogenous collateral that is used to collateralize DYAD can be 100%.
• Up to 50% of the collateral can be Kerosene.
• Kerosene's price is dependent on the other collateral present in the vaults and their price, meaning if the price of other collateral goes down, Kerosene will as well.
• Liquidations of accounts with smaller positions can be unattractive to liquidators due to the award being less than the gas fees.
• The same as above goes for positions which are close to a collateral ratio of 1 or below it.
• All of these can lead to bad debt accumulating in the system as well as DYAD being undercollateralized.

Proof of Concept

Currently, the way that the DYAD's Vault Manager / Kerosene Vaults are set up, it's prone to lead to the under-collateralization of assets, including a drop in the value of Kerosene as well, as its price is dependent on the price of other asset(s) in the vault.

Another thing which can serve as an attestation to this case is that the liquidation reward will be very unattractive to liquidators for users which:

• Have small positions (An example, a $1400 mint position, liquidated at a 1.13 CR will yield only $28 to the liquidator, which is going to be a lot less than the gas cost, especially during times of network congestion).
• No motivation to liquidate positions which are below 1e18 collateral ratio, leading to the accumulation of bad debt.
• The absence of any kind of a "minting" fee, which purpose will be to transfer a small part of the collateral deposit to a protocol-owned id so that there can always be a guarantee that Kerosene's value will be covered by some exogenous deposits.

When someone mints new DYAD, the only requirement for exogenous collateral is for its value to be greater than the amount of DYAD minted, which would mean above 100%, the general collateralization requirement only checks if the total collateral (exogenous + kerosine) is equal to 150%. Meaning that almost 50% of the collateral value can be kerosine:

  uint256 newDyadMinted = dyad.mintedDyad(address(this), id) + amount;
        if (getNonKeroseneValue(id) < newDyadMinted) revert NotEnoughExoCollat();
        dyad.mint(id, to, amount);
        if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();

The subsequent problem is that the value of Kerosene is dependent on the amount of exogenous collateral in the system and its value:

   uint tvl;
      address[] memory vaults = kerosineManager.getVaults();
      uint numberOfVaults = vaults.length;
      for (uint i = 0; i < numberOfVaults; i++) {
        Vault vault = Vault(vaults[i]);
        tvl += vault.asset().balanceOf(address(vault))
                * vault.assetPrice() * 1e18
                / (10**vault.asset().decimals())
                / (10**vault.oracle().decimals());
      }

• As only 100% of exogenous collateral is required, positions can get easily undercollateralized, especially during bear markets, black swan events and downward trends.
• Since the price of Kerosene is dependent on the amount and value of the exogenous collateral, the above-mentioned factors will also affect Kerosene leading to the further under-collateralization.
• Underwater positions which are unattractive to liquidators due to small fees and low collateral ratios will lead to the accumulation of bad debt in the system, further forwarding a depegging scenario.

Even though the system assumes that liquidations will be faster than the falling prices in a bear market, this might not always be true and a depegging event can be highly likely to occur in unfavorable market conditions.

Tools Used

Manual Review

Recommended Mitigation Steps

• Increase the requirement for exogenous collateral to at least 120%.
• Make sure to implement a small minting fee which will be allocated to the appropriate vault under a protocol-owned id so that there will always be some funds in the vaults to base the Kerosene's price upon.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

A lack of a minimum mint size doesn't incentivize users to liquidate small positions which can lead to the accrual of bad debt.

Proof of Concept

Considering that there isn't a minimum mint size and that DYAD is deployed on Ethereum, small positions which become undercollateralized have no incentive to be liquidated considering the gas costs on mainnet.

Here is an example of a mainnet transaction of a Liquidity Manager V1 liquidation: https://etherscan.io/tx/0x56f4bb05d02ddb8e82dfa1ef080c6b6b49e6670bff2671ff762d3072b94e3285

The transaction cost is $91.

This would mean that positions which are below a certain threshold wouldn't be sufficiently incentivized to be liquidated, especially during times of high network traffic on Ethereum, even more so when the price of ETH is on the higher end.

It can also incentivize holders of multiple NFTs to open smaller positions if that would present a lower chance of those positions being liquidated.

Tools Used

Manual Review

Recommended Mitigation Steps

Employ a minimum mint size across the different functions in order to enforce minimum mint requirement.

Assessed type

Invalid Validation