DYAD - ljj'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/src/core/KerosineManager.sol#L44-L51

Vulnerability details

Impact

Kerosene vault licensing is handled incorrectly, vulnerability this causes is that exogenous collateral vaults such as wETH vault will also be licensed as kerosene vaults allowing an attacker to double their collatRatio(), minting more tokens than they should be allowed which will lead to de-pegging of DYAD token.

Proof of Concept

If we look at Deploy.V2.s.sol following lines(64-65) add exogenous vaults into the KerosineManager.sol

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

This is done because the protocol wants to calculate the TVL in order to calculate the Kerosene asset price in the Vault.kerosine.unbounded.sol::assetPrice() function.

    function assetPrice() public view override returns (uint) {
        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 function will loop through the added exogenous vaults to get their values and calculate tvl.

If we look into how Kerosene vaults are added into VaultManagerV2.sol :

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

we see that function checks if the kerosene vaults are licensed in this line: if (!keroseneManager.isLicensed(vault)) revert VaultNotLicensed();. KerosineManager.sol checks for the license in the following function:

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

Since the deploy script added exogenous collateral vaults into the KerosineManager.sol contract this automatically means these vaults are also licensed by the KerosineManager.

An attack scenario happens like this:

1. Attacker will use this vulnerability to call VaultManagerV2.sol::add() and VaultManagerV2.sol::addKerosene() functions with wETH vault adding the same vault in seperate mappings.

mapping(uint => EnumerableSet.AddressSet) internal vaults;
mapping(uint => EnumerableSet.AddressSet) internal vaultsKerosene;

2. Attacker will deposit $100 worth of wETH into the wETH vault.
3. Attacker will call the mintDyad() function with amount = 99e18.

    function mintDyad(
        uint id,
        uint amount,
        address to
    ) external isDNftOwner(id) {
        uint 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();
        emit MintDyad(id, amount, to);
    }

4. if (getNonKeroseneValue(id) < newDyadMinted) check will be false and not revert (100e18<99e18)
5. Function will call dyad.mint() and mint 99 DYAD tokens.
6. Function will check the following line: if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();

   function collatRatio(uint id) public view returns (uint) {
        uint _dyad = dyad.mintedDyad(address(this), id);
        if (_dyad == 0) return type(uint).max;
        return getTotalUsdValue(id).divWadDown(_dyad);
    }
    
    function getTotalUsdValue(uint id) public view returns (uint) {
        return getNonKeroseneValue(id) + getKeroseneValue(id);
    }
    
    function getNonKeroseneValue(uint id) public view returns (uint) {
        uint totalUsdValue;
        uint numberOfVaults = vaults[id].length();
        for (uint i = 0; i < numberOfVaults; i++) {
            Vault vault = Vault(vaults[id].at(i));
            uint usdValue;
            if (vaultLicenser.isLicensed(address(vault))) {
                usdValue = vault.getUsdValue(id);
            }
            totalUsdValue += usdValue;
        }
        return totalUsdValue;
    }

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

7. Here we see that collatRatio() checks the getTotalUsdValue() function. Since the attacker has added exogenous collateral vaults as both normal vaults and kerosene vaults, both getNonKeroseneValue() and getKeroseneValue() functions will return 100e18, making total USD value 200e18.
8. collatRatio() will return 2e18 to the mintDyad() function, making the if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) return false(2e18 < 1.5e18) therefor it will not revert and transaction will complete successfully.The attacker now has minted 99e18 DYAD with only $100 worth of collateral when in reality they should need $200 worth of collateral to mint.

Tools Used

Manual review, Foundry

Recommended Mitigation Steps

Change the licensing of kerosene vaults. An example is shown below:

1. Create a new contract called KeroseneLicenser.sol that handles licensing of Kerosene vaults.

2. Delete KerosineManager.sol::isLicensed() function.

3. Change all keroseneManager.isLicensed(vault) lines in VaultManagerV2.sol into keroseneLicenser.isLicensed(vault).

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L95-L96

Vulnerability details

Impact

Licensing of kerosene vaults are not handled correctly in the deployment script, this will lead to breaking the math of collaterals in the protocol leading to many critical issues such as draining the protocol by minting more DYAD than exogenous collateral deposited.

Proof of Concept

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

Lines 93-96 in Deploy.V2.s.sol handles the licensing of the vaults which is an integral part of the protocol. However we can see that kerosine vaults are licensed with the vaultLicenser here aswell.

Vulnerability caused by this problem is that anyone can call the VaultManagerV2.sol::add() function for kerosine vaults aswell when that function should only work for exogeneous collateral vaults such as wETH. This leads to kerosine vaults being counted in getNonKeroseneValue() function shown below:

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

effectively leading users kerosines to be counted as exogeneous collateral effectively doubling the value of kerosines counted as collateral and kerosines being counted in functions where they should not be counted such as mintDyad() function as shown below:

  function mintDyad(
        uint id,
        uint amount,
        address to
    ) external isDNftOwner(id) {
        uint 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();
        emit MintDyad(id, amount, to);
    }

In an example scenario where a user has $100 worth of wETH as collateral and $100 worth of unbounded kerosines they should be able to only mint a maximum of 100e18 DYAD in the intended use. With this vulnerability attacker will put the amount in mintDyad() as 200e18, getNonKeroseneValue(id) will return $200 worth of tokens and will return false in the following if check: if (getNonKeroseneValue(id) < newDyadMinted). 200e18 DYAD will be minted to the attacker and function will then check for the next line: if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();. Here if we look at the collatRatio() function:

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

We can see that it returns the getTotalUsdValue(id).divWadDown(_dyad) value. If we look into the getTotalUsdValue() function:

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

We see that it is supposed to return values of collaterals in both vaults. However since kerosene vaults are also counted in getNonKeroseneValue() this function will return $200 + $100 worth of tokens and allowing the attacker to pass the collatRatio() check in mintDyad() resulting in the attacker being able to mint double the tokens that they should have minted.

Note that attacker was able to mint 200e18 DYAD while depositing only $100 worth of exogenous collateral so they can sell the DYAD in an AMM and keep repeating this process.

Tools Used

Manual review, foundry.

Recommended Mitigation Steps

Instead of using Licenser.sol for kerosene vaults, create a new contract that is KeroseneLicenser.sol which will only handle licensing of kerosene vaults. Then change the deploy script as shown below.

    vaultLicenser.add(address(ethVault));
    vaultLicenser.add(address(wstEth));
    keroseneVaultLicenser.add(address(unboundedKerosineVault));
    // keroseneVaultLicenser.add(address(boundedKerosineVault));

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

deposit() function lacking access control leads to DoS. Attacker will frontrun any withdraw() call with a deposit() call to DoS the withdraw() call, causing it to revert.

Proof of Concept

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

Shown above is the VaultManagerV2.sol::withdraw() function which has a flash loan protection in the code snippet shown below:

 if (idToBlockOfLastDeposit[id] == block.number)
            revert DepositedInSameBlock();

However if we look at where idToBlockOfLastDeposit[id] is updated, VaultManagerV2.sol::deposit() function is the one that updates idToBlockOfLastDeposit[id]

function deposit(
        uint id,
        address vault,
        uint amount
    ) external isValidDNft(id) {
        idToBlockOfLastDeposit[id] = block.number;
        Vault _vault = Vault(vault);
        _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
        _vault.deposit(id, amount);
    }

Problem here is that deposit() function lacks the isDNftOwner(id) modifier, meaning anyone can call this function with 1 token to update that dNFT’s idToBlockOfLastDeposit[id].

Example attack happens like this:

1. A normal user calls withdraw() to withdraw their collateral from their vault.

2. Attacker will see this call and frontrun it with their own deposit() call sending it in the same block, depositing 1 token into the normal users vault. (wETH and wstETH are 18 decimal tokens so 1 token is not worth anything.)

3. deposit() call will update the idToBlockOfLastDeposit[id] with the current block.number.

4. Normal users withdraw() call will revert when it reaches the if (idToBlockOfLastDeposit[id] == block.number) check because block.number will be the same.

Since this attack costs almost nothing to the attacker, attacker will keep frontrunning to permanently DoS the user.

Tools Used

Manual review.

Recommended Mitigation Steps

Add the isDNftOwner(id) modifier in the deposit() function as shown below:

function deposit(
        uint id,
        address vault,
        uint amount
    ) external isValidDNft(id) isDNftOwner(id) {
        idToBlockOfLastDeposit[id] = block.number;
        Vault _vault = Vault(vault);
        _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
        _vault.deposit(id, amount);
    }

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Unbounded Kerosine tokens are withdrawable in the intended use of the protocol. However a vulnerability in VaultManagerV2.sol causes them to be stuck in the contract. Difference of unbounded and bounded kerosine tokens are that bounded ones count as twice the value when collatRatio() of a user is being calculated with the downside of not being able to withdraw them, this vulnerability causes unbounded tokens to have the same downside while not having the upside of counting double which will lead to users tokens being forever stuck in the vault. Same vulnerability causes redeemDyad() function to revert as well.

Proof of Concept

If we look at the Vault.kerosine.unbounded.sol::withdraw():

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

we can see that it is only meant to be called by the VaultManagerV2.sol because of the onlyVaultManager modifier found in Vault.kerosine.sol.

  modifier onlyVaultManager() {
    if (msg.sender != address(vaultManager)) revert NotVaultManager();
    _;
  }

This means only way a user can withdraw their unbounded kerosine tokens is calling the withdraw() function in VaultManagerV2.sol.

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

However when a user tries to withdraw their kerosene token, entering their kerosene token vault as the address, following line in withdraw() will try to calculate the worth of the kerosene tokens in this line: uint value = (amount * _vault.assetPrice() * 1e18) / 10 ** _vault.oracle().decimals() / 10 ** _vault.asset().decimals();. Due to a lack of oracle in Vault.kerosine.unbounded.sol the transaction will fail when the function tries to calculate the value. This vulnerability also causes withdraw() calls to Vault.kerosine.bounded.sol to revert with the incorrect error.

Same vulnerability will happen in redeemDyad() function as well:

    function redeemDyad(
        uint id,
        address vault,
        uint amount,
        address to
    ) external isDNftOwner(id) returns (uint) {
        dyad.burn(id, msg.sender, amount);
        Vault _vault = Vault(vault);
        uint 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;
    }

Kerosene vaults not having an oracle will cause this function to revert.

Tools Used

Manual review, Foundry.

Recommended Mitigation Steps

Implement a new withdrawUnboundedKerosene() function in VaultManagerV2.sol

function withdrawUnboundedKerosene(
        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() / 10 ** _vault.asset().decimals();
        _vault.withdraw(id, to, amount);
        if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();
    }

Implement a new redeemDyadWithKerosene() function in VaultManagerV2.sol

    function redeemDyadWithKerosene(
        uint id,
        address vault,
        uint amount,
        address to
    ) external isDNftOwner(id) returns (uint) {
        dyad.burn(id, msg.sender, amount);
        Vault _vault = Vault(vault);
        uint value = (amount * _vault.assetPrice() / 10 ** _vault.asset().decimals();
        withdraw(id, vault, asset, to);
        emit RedeemDyad(id, vault, amount, to);
        return asset;
    }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Kerosine vaults are not counted for in VaultManagerV2.sol::liquidate() function which will lead to the function sending incorrect amount to liquidators causing a loss of funds for the liquidators. This leads will lead to liquidators avoiding calling liquidate() for users that keep part of their collateral in kerosine vaults, causing the protocol to accumulate bad debt and de-pegging the DYAD token.

Proof of Concept

    function liquidate(
        uint id,
        uint to
    ) external isValidDNft(id) isValidDNft(to) {
        uint cr = collatRatio(id);
        if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();
        dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));

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

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

As we can see in the liquidate() function above, the function will loop(Lines 221-226) through the vaults to send liquidated users collateral to the liquidator. However we can see in the contracts mappings that vaults and kerosineVaults are stored separately (Lines 34-35).

  mapping (uint => EnumerableSet.AddressSet) internal vaults; 
  mapping (uint => EnumerableSet.AddressSet) internal vaultsKerosene; 

Since the liquidate function only looks for exogenous vaults and their assets to liquidate and send to the liquidator and does not loop through the kerosene vault this will break the functions logic resulting in liquidate() function sending the liquidator less than the collateral amount they should get. This vulnerability disincentivizes liquidators which in turn will lead to bad debt accumulating in the contract and de-pegging the DYAD token.

Tools Used

Manual review, foundry

Recommended Mitigation Steps

Account for the keroseneVaults in the liquidate() function and update it as shown below:

    function liquidate(
        uint id,
        uint to
    ) external isValidDNft(id) isValidDNft(to) {
        uint cr = collatRatio(id);
        if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();
        dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));

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

        uint numberOfVaults = vaults[id].length();
        uint numberOfKeroseneVaults = keroseneVaults[id].lenght();
        for (uint i = 0; i < numberOfVaults; i++) {
            Vault vault = Vault(vaults[id].at(i));
            uint collateral = vault.id2asset(id).mulWadUp(
                liquidationAssetShare
            );
            vault.move(id, to, collateral);
        }
        for (uint i = 0; i < numberOfKeroseneVaults; i++) {
            Vault keroseneVault = Vault(keroseneVaults[id].at(i));
            uint collateral = keroseneVault.id2asset(id).mulWadUp(
                liquidationAssetShare
            );
            keroseneVault.move(id, to, collateral);
        }
        emit Liquidate(id, msg.sender, to);

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Introduction of bounded Kerosine Vaults will break liquidation logic, will cause the protocol to accumulate bad debt and go underwater, will de-peg the DYAD token and tank the value of Kerosene tokens.

Proof of Concept

First of we need to understand the difference of bounded kerosine compared to unbounded kerosine. This is explained in the Vault.kerosine.bounded.sol::assetPrice()

  function assetPrice() 
    public 
    view 
    override
    returns (uint) {
      return unboundedKerosineVault.assetPrice() * 2;
  }

As we can see bounded kerosines are twice as more valuable as unbounded kerosines however they cannot be withdrawn because of the following code in Vault.kerosine.bounded.sol::withdraw()

  function withdraw(
    uint    id,
    address to,
    uint    amount
  ) 
    external 
    view
      onlyVaultManager
  {
    revert NotWithdrawable(id, to, amount);
  }

Which essentially means that only use of bounded kerosine tokens are improving ones collatRatio()

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

This allows users to mint more dyad in VaultManagerV2.sol::mintDyad() function. For example a user can deposit $101 worth of wETH and $24.5 worth of bounded Kerosene tokens to mint 100 DYAD tokens while without the bounded Kerosene they would need to deposit $150 worth of wETH as we can see in the mintDyad() function.

    function mintDyad(
        uint id,
        uint amount,
        address to
    ) external isDNftOwner(id) {
        uint 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();
        emit MintDyad(id, amount, to);
    }

Problem starts when we look into the monetary gain of a liquidator when they call VaultManagerV2.sol::liquidate() function. With the exogeneous collateral vaults and unbounded kerosine vaults, liquidator will successfully get the amount they minted back and a 20% bonus. In an example scenario where UserA is the liquidator and UserB is the user with the bad debt.

1. UserB’s collateral value goes down to $149 worth of tokens while they have 100 DYAD tokens minted and becomes eligible to be liquidated.
2. UserA calls liquidate() with UserB as the target.
3. UserA burns 100 DYAD in the liquidate function.
4. UserA will get approximately 73% of UserB’s tokens which equates to approximately $110 worth of tokens.
5. UserA has made approximately $10 profit from calling the liquidate() function and paying the bad debt of UserB.

However in a situation where UserB has used bounded kerosine tokens this changes, since UserB can mint 100 DYAD with $101 worth of exogeneous collateral and $24.5 worth of bounded kerosine tokens. Let’s explore this situation.

1. UserB’s exogeneous collateral value goes down to $100 worth of tokens while they have 100 DYAD tokens minted and becomes eligible to be liquidated.

2. UserA calls liquidate() with UserB as the target.

3. UserA burns 100 DYAD in the liquidate function.

4. UserA will get approximately 73% of UserB’s tokens which equates to approximately $73 worth of exogeneous collateral tokens and approximately $17.8 worth of bounded kerosine.

5. UserA has burnt 100 DYAD tokens while getting $73 worth of useable collateral and is currently in a deficit of $27 since bounded kerosine can not be withdrawn.

Only way for UserA to make use of these bounded kerosine tokens is depositing more exogeneous collateral into the protocol to mint DYAD. At the current situation after liquidating, UserA can only mint 73 DYAD. In order to make use of the bounded collateral and make back the 100 DYAD they have burnt, UserA will need to deposit approximately $41 worth of exogeneous collateral making their total cost in this liquidation $141 just to get back the 100 DYAD they have burnt.

Problem created with bounded kerosine vaults is that any user that has these tokens will never be profitable for a liquidator to liquidate. This would cause bad debt in the protocol to accumulate, therefor DYAD’s overcollateralization will keep decreasing which in turn tanks the Kerosine token value since it is tied to the overcollateralization of DYAD.

Tools Used

Manual review, Foundry.

Recommended Mitigation Steps

Do not use Vault.kerosine.bounded.sol. If bounded vault will be used, implement new ways to reward Liquidators that are getting bounded Kerosine as collateral to create incentive for liquidation.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

When a user has received any bounded Kerosene tokens and has the bounded Kerosene vault added to their Note, It will be impossible for the user to remove the vault from their Note.

Proof of Concept

    function removeKerosene(uint id, address vault) external isDNftOwner(id) {
        if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();
        if (!vaultsKerosene[id].remove(vault)) revert VaultNotAdded();
        emit Removed(id, vault);
    }

VaultManagerV2.sol::removeKerosene() has the following check: if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();. This check is put so users can not accidentally remove any vaults they have assets in. However bounded Kerosene Vault has no ways of moving these tokens. Withdraw is not allowed because of the nature of the vault and Vault.kerosine.sol::move() has access control where only vault manager can access it:

  function move(
    uint from,
    uint to,
    uint amount
  )
    external
      onlyVaultManager
  {
    id2asset[from] -= amount;
    id2asset[to]   += amount;
    emit Move(from, to, amount);
  }

Meaning once a user has added the vault into their Note they won’t have any ways of calling removeKerosene() on it since it will always have assets and function will always revert at: if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();. Considering VaultManagerV2.sol has the following limitation on Kerosene vaults that can be added:

uint public constant MAX_VAULTS_KEROSENE = 5;

This poses a risk where a user might want to stop using bounded vaults and add new vaults into their Note. Only way user can remove their bounded kerosene from their account and be able to remove the bounded Kerosene vault is if they get fully liquidated.

Tools Used

Manual review, Foundry.

Recommended Mitigation Steps

Implement a new move() function for Vault.kerosine.bounded.sol or remove the access control in Vault.kerosine.sol::move().

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

burnDyad() function lacking access control leads to DoS. Attacker will frontrun any redeemDyad() function calls with a burnDyad() call to DoS the redeemDyad() call.

Proof of Concept

    function burnDyad(uint id, uint amount) external isValidDNft(id) {
        dyad.burn(id, msg.sender, amount);
        emit BurnDyad(id, amount, msg.sender);
    }

VaultManagerV2.sol::burnDyad() function lacks access control and an attacker will be able to call this function. An example attack scenario is shown below:

1. A normal user calls redeemDyad() function with amount = all their balance(Lets say 100e18 for simplicity).
2. Attacker seeing this call will then call burnDyad() function for 1 DYAD (DYAD has 1e18 decimals so 1 DYAD does not have any value) for the normal users vault.
3. burnDyad() function will call dyad.burn() with paremeters being id = normal users dNFT id, msg.sender = the vault, amount = 1

  function burn(
      uint    id, 
      address from,
      uint    amount
  ) external 
      licensedVaultManager 
    {
      _burn(from, amount);
      mintedDyad[msg.sender][id] -= amount;
  }

1. Here Dyad.sol::burn() function will reduce mintedDyad[msg.sender][id] by 1.
2. Attackers frontrun call finished and normal users redeemDyad() function reaches the first line where it will call burn() with amount = 100e18, however since the attacker has reduced mintedDyad[msg.sender][id] by 1, normal users call with revert due to an underflow error (99.99…e18 - 100e18).

Attacker will permanently DoS any users redeemDyad() call with barely any cost as long as the user tries to redeem all their balances.

Tools Used

Manual review.

Recommended Mitigation Steps

Implement access control to burnDyad() function as shown below:

function burnDyad(uint id, uint amount) external isValidDNft(id) isDNftOwner(id) {
        dyad.burn(id, msg.sender, amount);
        emit BurnDyad(id, amount, msg.sender);
    }

Assessed type

DoS