DYAD - KupiaSec's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Deploy.V2.s.sol#L95

Vulnerability details

Impact

In the DeployV2.run() function, the unboundedKerosineVault is added into the vaultLicenser, so the malicious DNftOwner can add the unboundedKerosineVault into vaults[id]. If the unboundedKerosineVault is added into the vaults[id], the getNonKeroseneValue() function is miscalculated. The malicious DNftOwner can mint more dyad tokens and withdraw vault tokens than actual amount. Also, the liquidation cannot be done fairly. Consequently, dyad token cannot stay stablity.

• DNftOwner can mint more dyad tokens and withdraw vault tokens than actual amount.
• The liquidation cannot be done fairly.
• Consequently, dyad token cannot stay stablility.

Proof of Concept

In the DeployV2.run() function, unboundedKerosineVault is added into vaultLicenser from L95.

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

File: script\deploy\Deploy.V2.s.sol
95:     vaultLicenser.add(address(unboundedKerosineVault));

So the malicious DNftOwner can add unboundedKerosineVault into vaults[id] from L76.

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

File: src\core\VaultManagerV2.sol
75:     if (!vaultLicenser.isLicensed(vault))  revert VaultNotLicensed();
76:     if (!vaults[id].add(vault))            revert VaultAlreadyAdded();

If malicious DNftOwner adds unboundedKerosineVault into vaults[id], the getNonKeroseneValue() function calculates USD value of real non-kerosene vaults and unboundedKerosineVault.

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

File: src\core\VaultManagerV2.sol
250:   function getNonKeroseneValue(
251:     uint id
252:   ) 
253:     public 
254:     view
255:     returns (uint) {
256:       uint totalUsdValue;
257:       uint numberOfVaults = vaults[id].length(); 
258:       for (uint i = 0; i < numberOfVaults; i++) {
259:         Vault vault = Vault(vaults[id].at(i));
260:         uint usdValue;
261:         if (vaultLicenser.isLicensed(address(vault))) {
262:           usdValue = vault.getUsdValue(id);        
263:         }
264:         totalUsdValue += usdValue;
265:       }
266:       return totalUsdValue;
267:   }

Thus, the return value of the getNonKeroseneValue() function is greater than real non-kerosene value. This makes the collatRatio greater than actual value. As a result, the malicious DNftOwner can mint more dyad tokens and withdraw vault tokens than actual amount. Also, the liquidation cannot be done fairly. Consequently, the dyad token cannot stay stable.

Tools Used

Manual Review

Recommended Mitigation Steps

In the DeployV2.run() function, kerosine vaults must not be added into vaultLicenser.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Attackers can delay any withdrawing and vault removing by depositing 1 wei.

Proof of Concept

At L143 of VaultManagerV2.withdraw(), there is a block.number check.

    function withdraw(
      uint    id,
      address vault,
      uint    amount,
      address to
    ) 
      public
        isDNftOwner(id)
    {
143   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(); 
    }

When someone tries withdrawing from his id, the attacker can set idToBlockOfLastDeposit[id] to the current block number by front-depositing 1 wei to that id, results in revert the withdrawal.

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

This DOS attack can be conducted to redeemDyad(), remove() and removeKerosene().

Tools Used

Manual review

Recommended Mitigation Steps

Deposit should be allowed to only the owner of DNFT token.

    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/main/src/core/VaultManagerV2.sol#L134-L153

Vulnerability details

Impact

Users can't withdraw kerosene and withdrawing from unadded vaults could be unfairly reverted.

Proof of Concept

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

    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 
148                 / 10**_vault.oracle().decimals() 
                    / 10**_vault.asset().decimals();
150   if (getNonKeroseneValue(id) - value < dyadMinted) revert NotEnoughExoCollat();
      _vault.withdraw(id, to, amount);
      if (collatRatio(id) < MIN_COLLATERIZATION_RATIO)  revert CrTooLow(); 
    }

If the vault is a kerosene vault, VaultManagerV2.withdraw() will be reverted at L148 because there is no oracle in kerosene vaults. And, if the vault was not added to id, value is not included to getNonKeroseneValue(id), so getNonKeroseneValue(id) should not be subtracted by value at L150. So L150 could result in a reversal of the transaction.

Tools Used

Manual review

Recommended Mitigation Steps

The collateral check should be improved.

    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();
+       uint value;
+       if (vaults[id].contains(vault) && vaultLicenser.isLicensed(address(vault)))
+       {
+           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(); 
    }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

Anyone can prevent liquidation.

Proof of Concept

There is only collateral ratio check at L214 in VaultManagerV2.liquidate().

    function liquidate(
        uint id,
        uint to
    ) 
        external 
        isValidDNft(id)
        isValidDNft(to)
        {
        uint cr = collatRatio(id);
214     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);
    }

But, anyone who is facing liquidation can deposit a large amount of collateral which results in the increasing of the kerosene price. Consequently, he can increase collateral ratio increase enough to avoid liquidation. Then, even though there is a less collateral than dyad in id, that id avoids liquidation.

Tools Used

Manual review

Recommended Mitigation Steps

The liquidation check should be improved.

    function liquidate(
      uint id,
      uint to
    ) 
      external 
        isValidDNft(id)
        isValidDNft(to)
      {
        uint cr = collatRatio(id);
-       if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();
+       if (cr >= MIN_COLLATERIZATION_RATIO && getNonKeroseneValue(id) >= dyad.mintedDyad(address(this), id)) 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);
    }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The asset price of unbounded kerosene vaults is calculated using the degree of DYAD’s overcollateralization, tvl - dyad.totalSupply(). If undercollateralization occurs, the subtract operation is reverted. This makes the calling of assetPrice() function reverted, so liquidation cannot be done.

In case of undercollateralization like sudden price drop of collateral assets, liquidation cannot be done.

Proof of Concept

In the Vault.kerosine.unbounded.assetPrice() function, the asset price of unbounded kerosene vaults is calculated using the degree of DYAD’s overcollateralization from L65.

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

File: src\core\Vault.kerosine.unbounded.sol
58:       for (uint i = 0; i < numberOfVaults; i++) {
59:         Vault vault = Vault(vaults[i]);
60:         tvl += vault.asset().balanceOf(address(vault)) 
61:                 * vault.assetPrice() * 1e18
62:                 / (10**vault.asset().decimals()) 
63:                 / (10**vault.oracle().decimals());
64:       }
65:       uint numerator   = tvl - dyad.totalSupply();
66:       uint denominator = kerosineDenominator.denominator();

Here, tvl is the total USD value of kerosineManager's vaults. If the asset price of kerosineManager's vaults drops, tvl becomes smaller than dyad.totalSupply(). This leads the subtract operation to be reverted from L65. Finally, in the case of undercollateralization, nobody can not call Vault.kerosine.unbounded.assetPrice() function. However anyone must be allowed to liquidate that DNft.

In the VaultManagerV2, calculating collatRatio needs to call Vault.kerosine.unbounded.assetPrice() function.

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

File: src\core\VaultManagerV2.sol
230:   function collatRatio(
231:     uint id
232:   )
233:     public 
234:     view
235:     returns (uint) {
236:       uint _dyad = dyad.mintedDyad(address(this), id);
237:       if (_dyad == 0) return type(uint).max;
238:       return getTotalUsdValue(id).divWadDown(_dyad);
239:   }
240: 
241:   function getTotalUsdValue(
242:     uint id
243:   ) 
244:     public 
245:     view
246:     returns (uint) {
247:       return getNonKeroseneValue(id) + getKeroseneValue(id);
248:   }
       [...]
269:   function getKeroseneValue(
270:     uint id
271:   ) 
272:     public 
273:     view
274:     returns (uint) {
275:       uint totalUsdValue;
276:       uint numberOfVaults = vaultsKerosene[id].length(); 
277:       for (uint i = 0; i < numberOfVaults; i++) {
278:         Vault vault = Vault(vaultsKerosene[id].at(i));
279:         uint usdValue;
280:         if (keroseneManager.isLicensed(address(vault))) {
281:           usdValue = vault.getUsdValue(id);        
282:         }
283:         totalUsdValue += usdValue;
284:       }
285:       return totalUsdValue;
286:   }

Calculating the collatRatio() function from L213 is always reverted.

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

File: src\core\VaultManagerV2.sol
205:   function liquidate(
206:     uint id,
207:     uint to
208:   ) 
209:     external 
210:       isValidDNft(id)
211:       isValidDNft(to)
212:     {
213:       uint cr = collatRatio(id);
           [...]
228:   }

Consequently, liquidation cannot be done.

Tools Used

Manual Review

Recommended Mitigation Steps

File: src\core\Vault.kerosine.unbounded.sol
  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());
      }
+     if (tvl < dyad.totalSupply())
+         return 0;
      uint numerator = tvl - dyad.totalSupply();
      uint denominator = kerosineDenominator.denominator();
      return numerator * 1e8 / denominator;
  }

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

In liquidation, the liquidator burns a quantity of DYAD equal to the target Note’s DYAD minted balance, and in return receives an equivalent value plus a 20% bonus of the target Note’s backing colateral.(check here) However, since Kerosene used as collateral in the target Note are not moved to the liquidator, it discourages liquidation and the liqudator migth receive less incentives even when the target Note's collateral ratio is bigger than 1.

Proof of Concept

When a liqudator liquidates the target Note, he burns a quantity of DYAD equal to the target Note’s DYAD minted balance and in return receives an equal value plus a 20% bonus of the target Note's backing collateral.

But in the VaultManagerV2.liquidate() function, only the Non-Kerosine part of collateral is directed at L221.

File: 2024-04-dyad\src\core\VaultManagerV2.sol
205:   function liquidate(
206:     uint id,
207:     uint to
208:   ) 
209:     external 
210:       isValidDNft(id)
211:       isValidDNft(to)
212:     {
213:       uint cr = collatRatio(id);
214:       if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();
215:       dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));
216: 
217:       uint cappedCr               = cr < 1e18 ? 1e18 : cr;
218:       uint liquidationEquityShare = (cappedCr - 1e18).mulWadDown(LIQUIDATION_REWARD);
219:       uint liquidationAssetShare  = (liquidationEquityShare + 1e18).divWadDown(cappedCr);
220: 
221:       uint numberOfVaults = vaults[id].length(); // @audit-info Only non-Kerosene collaterals are considered.
222:       for (uint i = 0; i < numberOfVaults; i++) {
223:           Vault vault      = Vault(vaults[id].at(i));
224:           uint  collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);
225:           vault.move(id, to, collateral);
226:       }
227:       emit Liquidate(id, msg.sender, to);
228:   }

As Kerosene is used as the collaterals, in the liquidation process, the asset of KeroseneVaults should be moved to the liquidator.

Scenario of the issue

Assume Alice is going to liquidate Bob's Note. Let Bob's Note is as follows.

In this case, his collateral ratio is larger than 100 % but smaller than 150%.

At L213-L219, the variables cr, cappedCr, liquidationEquityShare, liquidationAssetShare are calculated as below.

• cr = 62280.6 / 6000 = 1.1213 * 1e18
• cappedCr = max(1, cr) = 1.1213 * 1e18
• liquidationEquityShare = (cappedCr - 1e18).mulWadDown(0.2e18) = 0.0242 * 1e18
• liquidationAssetShare = (liquidationEquityShare + 1e18).divWadDown(cappedCr) = 0.9134 * 1e18

Then, the UsdValue of insentive of liquidator is calculated like below.

To sum up, the liquidator burned DYAD of $60000 but received only $56888.97 as incentive. Although Alice liquidated Bob's Note, she received less incentive than her waste. This is not fair for liquidators who contributes to enhance the security of the protocol.

The reason behind this is that the protocol allowed to move Non-Kerosene assets only in liquidation process although Kerosene was used to calculate Bob's collateral ratio. If the protocol allows to move Kerosene, it will be fair. It is revealed below.

But still, if collateral ratio drops below 1, the liqudator always receive less incentives than the amount she wasted to liquidate.

Tools Used

Manual Review

Recommended Mitigation Steps

In VaultManagerV2.liquidate(), assets of vaultsKerosene should be moved to the liquidator.

File: 2024-04-dyad\src\core\VaultManagerV2.sol
205:   function liquidate(
206:     uint id,
207:     uint to
208:   ) 
209:     external 
210:       isValidDNft(id)
211:       isValidDNft(to)
212:     {
213:       uint cr = collatRatio(id);
214:       if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();
215:       dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));
216: 
217:       uint cappedCr               = cr < 1e18 ? 1e18 : cr;
218:       uint liquidationEquityShare = (cappedCr - 1e18).mulWadDown(LIQUIDATION_REWARD);
219:       uint liquidationAssetShare  = (liquidationEquityShare + 1e18).divWadDown(cappedCr);
220: 
221:       uint numberOfVaults = vaults[id].length();
222:       for (uint i = 0; i < numberOfVaults; i++) {
223:           Vault vault      = Vault(vaults[id].at(i));
224:           uint  collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);
225:           vault.move(id, to, collateral);
226:       }

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

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

A malicious attacker can manipulate the price of Kerosene by depositing or transfering large amount of Non-Kerosene tokens to the vaults.

• Depositing or transfering Non-Kerosene tokens to the vaults increases the tvl of the protocol and also the price of Kerosene. It makes it difficult to liquidate the Note with KerosenVaults due to the rise of collateral ratio.
• The attacker withdraws all deposited tokens when the price of the tokens decreases. As a result, the price of Kerosene decreases and he can place the Notes with KeroseneVaults in the liquidation state. The attacker liquidates the Notes and can earn yields from the liquidation.

Proof of Concept

The price of Kerosene is calculated from the Vault.kerosine.unbounded.assetPrice() function,

File: 2024-04-dyad\src\core\Vault.kerosine.unbounded.sol
50:   function assetPrice() 
51:     public 
52:     view 
53:     override
54:     returns (uint) {
55:       uint tvl;
56:       address[] memory vaults = kerosineManager.getVaults();
57:       uint numberOfVaults = vaults.length;
58:       for (uint i = 0; i < numberOfVaults; i++) {
59:         Vault vault = Vault(vaults[i]);
60:         tvl += vault.asset().balanceOf(address(vault)) //@audit-info avoid to use balanceOf(address(vault))
61:                 * vault.assetPrice() * 1e18
62:                 / (10**vault.asset().decimals()) 
63:                 / (10**vault.oracle().decimals());
64:       }
65:       uint numerator   = tvl - dyad.totalSupply();
66:       uint denominator = kerosineDenominator.denominator();
67:       return numerator * 1e8 / denominator;
68:   }

First, it calculates tvl of the protocol. Here, it uses the balance of vault at L60.

An attacker can manipulate the balance of vault by transfering large amount of Non-Kerosine tokens to it directly via ERC20.Safetransfer or depositing via VaultManagerV2.deposit()

File: 2024-04-dyad\src\core\VaultManagerV2.sol
119:   function deposit(
120:     uint    id,
121:     address vault,
122:     uint    amount
123:   ) 
124:     external 
125:       isValidDNft(id)
126:   {
127:     idToBlockOfLastDeposit[id] = block.number;
128:     Vault _vault = Vault(vault);
129:     _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
130:     _vault.deposit(id, amount);
131:   }

It makes it difficult to liquidate the Note with KeroseneVaults. When tvl decreases due to price fluctuation, he withdraws deposited assets and place Note with KeroseneVaults in the liqudation state. Then he can earns yields by liquidating all available Notes

Scenario of the issue

Alice is going to manipulate the price of Kerosene and wants to earn yields.

• She mints Note by calling DNft.mintNft().
• She never mints Dyad from the Note, as a result, her Note's collateral ratio is uint.max and she can withdraw her Note's collateral assets at anytime as much as she want.
• She deposits 10000 ether of WETH to ethVault via the minted Note. This increases the balance of WETH of ethVault, tvl of the protocol. Consequencely, the price of Kerosene increases.
• First, this makes it difficult to liquidate the Notes with KeroseneVaults balance as their collateral ratios rise.
• When the price of WETH decreases, she decided to withdraw all deposited WETH. It results the decrease of the price of Kerosene, the decrease of the collateral ratio of the Notes with KeroseneVaults. Then, the Notes with KeroseneVaults are placed in the liquidation state.
• She liquidates the Notes and earn yields from liquidation.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the to avoid using KeroseneVaults in the Notes.

And we also recommed to use internal accounting for collaterals.

Assessed type

Other