DYAD - AlexCzm's results

The first capital efficient overcollateralized stablecoin.

General Information

Platform: Code4rena

Start Date: 18/04/2024

Pot Size: $36,500 USDC

Total HM: 19

Participants: 183

Period: 7 days

Judge: Koolex

Id: 367

League: ETH

DYAD

Findings Distribution

Researcher Performance

Rank: 49/183

Findings: 7

Award: $247.59

🌟 Selected for report: 0

🚀 Solo Findings: 0

Lines of code

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

Vulnerability details

Impact

DYAD depeg and protocol failure is iminent. VaultManagerV2.getNonKeroseneValue() should return TVL of exogenous collateral (different forms of staked ETH) locked with nftID. Instead a bigger value can be returned, because simple vaults can contain kerosene vaults too.

Proof of Concept

dNFT owners can add 2 types of collateral vaults to mint DYAD: exogenous, which are ETH based vaults and endogenous collateral vaults (based on kerosene). There are 2 mappings that keep track of which vault is added to which nftId:

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

The problem lies in the fact users can add any type of vault to vaults mapping, as long as the it's isLicensed :

  function add(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
//@audit nft owners can add kerosene vaults to ETH-based vaults mapping
    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);
  }

vaultLicenser whitelists both kerosene and eth based vaults as we can see in DeployV2 script: https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/script/deploy/Deploy.V2.s.sol#L93

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

getNonKeroseneValue returns (eth + kerosene) value locked with respective nftId:

  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))) {
// @audit return usdValue of all vaults: eth AND kerosene based
          usdValue = vault.getUsdValue(id);        
        }
        totalUsdValue += usdValue;
      }
      return totalUsdValue;
  }

getNonKeroseneValue is used in multiple places to ensure:

  • exogenous collateral locked >= than value of DYAD minted.
  • collateralRatio >= MIN_COLLATERIZATION_RATIO (1.5 overcollateralization).

But since getNonKeroseneValue returns an inflated value, both invariants are broken.

Tools Used

Manual review

KeroseneManager whitelists only exogenous collateral vaults. Add the following check to add function to ensure only exogenous vaults can be added to vaults mapping:

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


## Assessed type

Invalid Validation

#0 - c4-pre-sort

2024-04-29T06:38:22Z

JustDravee marked the issue as duplicate of #966

#1 - c4-pre-sort

2024-04-29T08:37:21Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-04T09:46:31Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-28T15:28:21Z

koolexcrypto marked the issue as duplicate of #1133

#4 - c4-judge

2024-05-29T14:03:07Z

koolexcrypto marked the issue as satisfactory

Lines of code

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

Vulnerability details

Impact

VaultManagerV2.getKeroseneValue returns the wrong collateral value. Instead of returning the kerosene value locked with respective nftID, it returns the value of exogenous (wETH, wstETH) collateral locked. DYAD can depegg from dollar.

Proof of Concept

dNFT owners can add 2 types of collateral vaults to mint DYAD: exogenous, which are ETH based vaults and endogenous collateral vaults (based on kerosene). These 2 mappings keep track of which vault is added to which nftId:

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

KeroseneManager role is to keep track of which vaults are used in Kerosene price calculation - only non-kerosene vaults are 'licensed'. https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/script/deploy/Deploy.V2.s.sol#L62-L65

//Deploy.V2.s.sol
...
   KerosineManager kerosineManager = new KerosineManager();
   kerosineManager.add(address(ethVault));
   kerosineManager.add(address(wstEth));
...

The problem is that only keroseneManager licensed vaults can be added to keroseneVaults:

  function addKerosene(uint id, address vault) external isDNftOwner(id) {
    if (vaultsKerosene[id].length() >= MAX_VAULTS_KEROSENE) revert TooManyVaults();
//@audit only ETH based vaults can  be added
    if (!keroseneManager.isLicensed(vault))                 revert VaultNotLicensed();
    if (!vaultsKerosene[id].add(vault))                     revert VaultAlreadyAdded();
    emit Added(id, vault);
  }

This is problematic because getKeroseneValue returns the value from vaultsKerosene list, which, as we saw, are ETH based vaults. Since totalUsdValue is calculated as keroseneValue + nonKeroseneValue, getNonKeroseneValue will returns 2x nonKeroseneValue.

The TVL > DYAD total supply invariant is broken because for each 100 USD worth of collateral deposited (wETH, wstETH), getTotalUsdValue will returns twice the value. Users will be able to mint 2x more DYAD than they should be able to (ignoring the 150% overcollateralization which is making even worse).

Tools Used

Manual review

Consider the following updates:

  function addKerosene(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
    if (vaultsKerosene[id].length() >= MAX_VAULTS_KEROSENE) revert TooManyVaults();
    // ensure it's a valid vault
+    if (!vaultLicenser.isLicensed(vault))  revert VaultNotLicensed();
-    if (!keroseneManager.isLicensed(vault))                 revert VaultNotLicensed();
     // ensure exogenous collateral vaults is not added
+    if (keroseneManager.isLicensed(vault))                 revert ExogenousCollateral();

    if (!vaultsKerosene[id].add(vault))                     revert VaultAlreadyAdded();
    emit Added(id, vault);
  }
...
  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))) {
        // ensure vault is stil licensed by admins 
+        if (vaultLicenser.isLicensed(address(vault))) {
          usdValue = vault.getUsdValue(id);        
        }
        totalUsdValue += usdValue;
      }
      return totalUsdValue;



## Assessed type

Invalid Validation

#0 - c4-pre-sort

2024-04-29T06:38:37Z

JustDravee marked the issue as duplicate of #966

#1 - c4-pre-sort

2024-04-29T08:37:25Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-04T09:46:30Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-28T15:28:22Z

koolexcrypto marked the issue as duplicate of #1133

#4 - c4-judge

2024-05-29T14:03:10Z

koolexcrypto marked the issue as satisfactory

Lines of code

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

Vulnerability details

Impact

Honest depositors can be denied from withdrawing their funds indefinitely.

Proof of Concept

VaultManagerV2.withdraw() can be easily DOSed. A malicious user can front-run a 'withdraw' request and deposit 1wei of collateral:

idToBlockOfLastDeposit is set to current block.number; https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L127

   function deposit(uint256 id, address vault, uint256 amount) external isValidDNft(id) {
        idToBlockOfLastDeposit[id] = block.number;
...

Then, the honest withdraw request is reverted if a deposit was done in same block.number: https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L143

    function withdraw(uint256 id, address vault, uint256 amount, address to) public isDNftOwner(id) {
        // @audit dos risc
        if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();
...

The attacker can continue this attack as long as he whishes (eg. waiting for collateral ratio to fall below liquidation threshold); The only cost incurred to bad actor is the gas fee, as deposit doesn't enforce a minimum deposit.

Tools Used

Manual review

Consider allowing only nftOwner to deposit under their nftID OR implement a mechanism to allow nftOwners to whitelists what addresses can deposit collateral under his nftID;

Assessed type

DoS

#0 - c4-pre-sort

2024-04-27T11:56:29Z

JustDravee marked the issue as duplicate of #489

#1 - c4-pre-sort

2024-04-29T09:25:44Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-05T20:39:59Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-05T20:45:36Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#4 - c4-judge

2024-05-05T21:52:07Z

koolexcrypto marked the issue as nullified

#5 - c4-judge

2024-05-05T21:52:10Z

koolexcrypto marked the issue as not nullified

#6 - c4-judge

2024-05-08T15:26:40Z

koolexcrypto marked the issue as duplicate of #1001

#7 - c4-judge

2024-05-11T19:49:07Z

koolexcrypto marked the issue as satisfactory

Awards

3.8221 USDC - $3.82

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
:robot:_28_group
duplicate-830

External Links

Lines of code

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

Vulnerability details

Impact

Users are unable to withdraw kerosene tokens from unbounded kerosene vault due to a call to a non-existent method;

Proof of Concept

deposit and withdrawals of kerosene from the UnboundedKerosineVault are executed through VaultManagerV2 because of the onlyVaultManager modifier attached to both functions.

The vaultManager.withdraw() function invokes several methods on the vault argument: _vault.assetPrice(), _vault.oracle(), and _vault.asset(). However, since the kerosene vaults lack an 'oracle' method, the transaction will fail, preventing users from withdrawing their kerosene tokens.

  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() // @audit kerosene vault lask an oracle method; tx reverts
                  / 10**_vault.asset().decimals();
...

Tools Used

Manual review

For vaults storage variable containing only exogenous collateral vaults (wETH, wstETH), consider updating following functions:

  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 = hasVault(id, vault) 
+        ? amount * _vault.assetPrice() 
+                  * 1e18 
+                  / 10**_vault.oracle().decimals() 
+                  / 10**_vault.asset().decimals()
+        : 0; // if kerosene vault, subtract 0
    if (getNonKeroseneValue(id) - value < dyadMinted) revert NotEnoughExoCollat();
    _vault.withdraw(id, to, amount);
    if (collatRatio(id) < MIN_COLLATERIZATION_RATIO)  revert CrTooLow(); 
  }

  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;

+      uint asset = hasVault(id, vault) 
+            ? amount * (10**(_vault.oracle().decimals() + + _vault.asset().decimals())) 
+                    / _vault.assetPrice() 
+                    / 1e18
+            : amount * 1e8/ _vault.assetPrice(); // kerosene vaults returns assetPrice() with 8 decimals precision 
      withdraw(id, vault, asset, to);
      emit RedeemDyad(id, vault, amount, to);
      return asset;
  }

-    function hasVault(uint256 id, address vault) external view returns (bool) {
+    function hasVault(uint256 id, address vault) public view returns (bool) {
        return vaults[id].contains(vault);
        
    }

Assessed type

Invalid Validation

#0 - c4-pre-sort

2024-04-26T21:29:04Z

JustDravee marked the issue as duplicate of #1048

#1 - c4-pre-sort

2024-04-28T18:38:32Z

JustDravee marked the issue as duplicate of #830

#2 - c4-pre-sort

2024-04-29T08:44:53Z

JustDravee marked the issue as sufficient quality report

#3 - c4-judge

2024-05-11T20:04:17Z

koolexcrypto marked the issue as satisfactory

Awards

7.3026 USDC - $7.30

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
upgraded by judge
:robot:_97_group
duplicate-128

External Links

Lines of code

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

Vulnerability details

Impact

In some cases liquidators are getting back less than the value of burned DYAD making liquidations unprofitable and increasing the risk of protocol solvency.

Proof of Concept

Before delving into the issue, some context first.

The sponsor made it clear in the discord chat that :

- vaults are for exogenous collateral - kerosene vaults are for kerosene, the token, endogenous to the protocol

This means that dNFT owners can call vaultManagerV2's add and addKerosene to enable exogenous collateral (wETH, wstETH) respectively kerosene collateral deposits under their dNFT.

There is a problem that allow nftOwners to add any type of vault (eg. wETH vaults and kerosene vaults) to VaultManagerV2 vaults mapping. This finding was reported in another issue and it's mentioned here only to give some more context.

After the above finding is fixed, in some conditions liquidations will be unprofitable.

In liquidate a liquidationAssetShare percentage is calculated. This represents the % of collateral liquidator should get: it includes the value of DYAD burned and a LIQUIDATION_REWARD incentive bonus. The problem is that only the exogenous collateral is moved to liquidator balance. In most cases the amounts received is smaller than the value of DYAD burned from their balance.

Let's take an example.

  • An user deposit wETH and kerosene worth > $150 and mints 100 DYAD
  • after some time his collateral is worth $100 in wETH and $49 in kerosene. His collatRatio < 1.5 and can get liquidated.
  • a liquidator calling the liquidate will burn 100 DYAD and get back:
liquidationAssetShare = ((cappedCr - 1e18) * LIQUIDATION_REWARD + 1e18) / cappedCr liquidationAssetShare = ((1.49 - 1) * 0.2 + 1) / 1.49 liquidationAssetShare = 1.098 / 1.49 ~= 0.7369

He will be rewarded with 73.69% of liquidated user collateral. But since vaults contains only exogenous collateral, liquidator will receive

0.7369 * (100$ worth of wETH) = 73.69$ worth of wETH

which is smaller than the value of DYAD burned.

Tools Used

Consider looping over keroseneVauls as well and move corresponding % of kerosene to liquidator nftId:

  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);
      }
+      uint numberOfKeroseneVaults = vaultsKerosene[id].length();
+      for (uint i = 0; i < numberOfKeroseneVaults; 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

Invalid Validation

#0 - c4-pre-sort

2024-04-28T10:19:16Z

JustDravee marked the issue as duplicate of #128

#1 - c4-pre-sort

2024-04-29T09:03:39Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-11T19:42:55Z

koolexcrypto marked the issue as satisfactory

#3 - c4-judge

2024-05-13T18:40:17Z

koolexcrypto changed the severity to 3 (High Risk)

Findings Information

🌟 Selected for report: SBSecurity

Also found by: AlexCzm, Emmanuel, Stefanov, carlitox477, carrotsmuggler, d3e4, grearlake, peanuts

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
:robot:_75_group
duplicate-982

Awards

223.9474 USDC - $223.95

External Links

Lines of code

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

Vulnerability details

Impact

In case of a liquidation, liquidator receive less than the 20% bonus of the target Note’s backing colateral.

Proof of Concept

According to documents, liquidator should receive a 20% bonus of the target note's backing collateral on top of the DYAD burned.

Liquidation and Redemption from docs :

If a Note’s collateral value in USD drops below 150% of its DYAD minted balance, it faces 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, which the liquidator can direct to any other Note, usually their own. The target keeps the remainder of their collateral, if any.

No matter if the bonus of the target Note’s backing colateral refers to the 150% overcollateralization target value or to the minimum of 100% colateral required to keep the DYAD peg, the liquidator receive less value than documents specify.

  /// @inheritdoc IVaultManager
  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;

      //@audit the 20% bonus is applied wrongly
      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);
  }

Let's take s simple example to check the math.

  • Alice deposit 150$ worth of collateral (wETH) and mints 100 DYAD. Her collatRatio >= MIN_COLLATERIZATION_RATIO = 150%.
  • after some time the wETH price drops and her collateral is now worth 149$, just slightly under the 150% min collateralization ratio.
  • Bob calls the liquidation method: -> he burns 100 DYAD -> he will get back the following amount of collateral: wethValueReceived = ((CR - 1e18) * LIQUIDATION_REWARD) + 1e18) / CR * CollateralValue

where CR = capped collateral ratio = 1.49e18 CollateralValue = 149 USD

wethValueReceived = (1.49e18 - 1e18) * 0.2e18) + 1e18) / 1.49e18 * 149 wethValueReceived = 1.098 / 1.49 * 149 wethValueReceived = 0.7369127516778523 * 149 wethValueReceived = 109.8

No matter if target note's backing collateral refers to minimum of collateral required to keep the DYAD peg position (eg. 100$ worth of collateral in above example) or the collateral amount required to keep the CR >= 150%, liquidator receive less than the 20% bonus.

The reward bonus percentage became smaller as the CR became smaller. For a position of 100 DYAD minted and 120$ worth of collateral we get a CR = 1.2: wethValueReceived = 1.04 / 1.2 * 120 = 104\$ received for burning 100 DYAD That's a 4% bonus.

Tools Used

If the LIQUIDATION_REWARD bonus should be applied to the amount of DYAD burned (eg. for 100 DYAD repaid, liquidator should get 120% of that amount in collateral), consider updating the liquidation function:

  /// @inheritdoc IVaultManager
  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);
       // if CR <= 120% then liquidator get all his collateral 
+      uint divisor = cr < 1.2e18 ? 1.2e18 : cr;
+      uint liquidationAssetShare  = (1e18 + LIQUIDATION_REWARD) * 1e18 / divisor;

      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

Math

#0 - c4-pre-sort

2024-04-29T06:39:41Z

JustDravee marked the issue as duplicate of #75

#1 - c4-pre-sort

2024-04-29T11:59:11Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-11T12:12:36Z

koolexcrypto marked the issue as satisfactory

#3 - c4-judge

2024-05-28T19:21:58Z

koolexcrypto marked the issue as duplicate of #982

Awards

7.3512 USDC - $7.35

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
:robot:_39_group
duplicate-118

External Links

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L101 https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L113 https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L119-L131

Vulnerability details

Impact

Nft owners can be blocked from removing the vaults associated with their dNFT. Since they can add a limited number of vaults ((MAX_VAULTS = MAX_VAULTS_KEROSENE = 5), they can't add additional vaults if they reached max_vaults.

Proof of Concept

An attacker can front run remove/ removeKerosene requests and deposit 1wei of asset to the vault to be removed. Vault balance is increased: https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/Vault.sol#L42-L51

// Vault.sol 

function deposit(
    uint id,
    uint amount
  )
    external 
      onlyVaultManager
  {
    id2asset[id] += amount;
    emit Deposit(id, amount);
  }

Because of following check the vault removal will revert: https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L94C1-L101C64

//VaultManagerV2.sol

  function remove(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
    if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();

dNFT owner can't remove the vault. This can impact the users in different ways:

  • if a vulnerability is found within the vault (new vaults can be deployed and added later);
  • users can add other hot new vaults if they already hit the max_vaults limit.

Due to 'function of the protocol or its availability' is impacted I consider M severity to be fair.

Tools Used

Manual review

Consider removing the id2asset(id) > 0 check and instead revert the tx if removal of vault makes the collateralRatio fall falls below threshold.

Assessed type

DoS

#0 - c4-pre-sort

2024-04-27T13:34:10Z

JustDravee marked the issue as duplicate of #489

#1 - c4-pre-sort

2024-04-29T09:25:44Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-05T20:39:59Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-05T20:45:36Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#4 - c4-judge

2024-05-05T21:51:34Z

koolexcrypto marked the issue as nullified

#5 - c4-judge

2024-05-05T21:51:38Z

koolexcrypto marked the issue as not nullified

#6 - c4-judge

2024-05-05T21:51:43Z

koolexcrypto marked the issue as not a duplicate

#7 - c4-judge

2024-05-06T08:56:26Z

koolexcrypto marked the issue as duplicate of #118

#8 - c4-judge

2024-05-11T12:23:40Z

koolexcrypto marked the issue as satisfactory

Awards

4.8719 USDC - $4.87

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
:robot:_67_group
duplicate-67

External Links

Lines of code

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

Vulnerability details

Impact

Bad actors can deposit exogenous collateral but mint no DYAD to maximize the kerosene price increase. Users will mint DYAD using kerosene at inflated price; When the exogenous collateral is removed, kerosene price decreases and collateral ratio will fall drastically, exposing users to liquidation risk.

Proof of Concept

As mentioned in docs, Kerosene price is not set by market but has a deterministic value given by formula:

$K_{\text{value}} = \frac{TVL - D_{\text{supply}}}{K_{\text{supply}}}$

where: $K_{\text{value}}$ -> kerosene price TVL -> total value of exogenous collateral locked in the protocol $D_{\text{supply}}$ -> total DYAD minted $K_{\text{supply}}$ -> circulating kerosene supply

Kerosene price formula starts from the assumption that all exogenous collateral (eg. wETH) locked in vaults ensure all DYAD open positions stability (by a 'DYAD open position' I mean the total DYAD minted through a dNFT id).

A malicious user can deposit exogenous collateral (let's say wETH) into a dNFT for the only purpose to increase kerosene price. When another user deposit kerosene as collateral to mint DYAD, malicious user will back-run it and withdraw the wETH. Kerosene price drops and liquidation likelihood is increased.

Let's take the following example. For the simplicity, the exogenous collateral (eg. wETH) volatility is ignored.

1.Initial conditions TVL = $1M in wETH DYAD supply = 600k; Kerosene circulating supply(let's note it Ksupply) = 50M

  1. Alice, a malicious user, deposit $400k in wETH

  2. Bob, our victim, deposit $100k in wETH The new TVL = 1000k + 400k + 100k = 1500k Kerosene price = (TVL - DYAD supply) / Ksupply = (1500k - 500k) / 50M = $0.02

  3. Bob deposit 5M kerosene, worth $100k

  4. Bob mint 100k DYAD Kerosene price = (TVL - (DYAD supply + DYAD to mint)) / Ksupply kerosene price = (1500k - 700k) / 50M = $0.016 Bob's CollateralRatio = 100k + (0.016 * 5M) / 100k = 180% (a relatively comfortable overcolateralization)

  5. Alice withdraw her wETH deposit TVL = 1500k - 400k = 1100k Kerosene price = (1100k - 700k) / 50M = $\0.008

  6. Bob's CR = (100K + 0.008 * 5M) / 100k = 140% < MIN_COLLATERIZATION_RATIO = 150% Bob's position can be liquidated.

The same mechanism can be used for different purposes. Eg. front-run a liquidation tx and deposit exogenous collateral into a different dNFT he owns. Kerosene price is increased and user avoid being liquidated even if he did not deposited more collateral to his position.

If we check the deployed contracts we can see that the numbers from this example are similar to values from deployed contracts:

MAINNET_WETH_VAULT -> $1.22 M worth of wETH https://etherscan.io/address/0xcF97cEc1907CcF9d4A0DC4F492A3448eFc744F6c

MAINNET_KEROSENE -> 1_000_000_000 total supply, ~950M hold by MAINNET_OWNER => 50M in circulation https://etherscan.io/token/0xf3768D6e78E65FC64b8F12ffc824452130BD5394#readContract

MAINNET_DYAD -> 632k DYAD https://etherscan.io/token/0x305B58c5F6B5b6606fb13edD11FbDD5e532d5A26

Tools Used

Manual review

I see this issue as a design flaw. Users can abuse it with the only down side to lock exogenous collateral for 1 block (+gas fees).

An ideea to mitigate this price manipulation is locking the collateral for more than one block. Bad actors will not be able to decrease kerosene price on request, unless X block have passed since deposit.

Assessed type

MEV

#0 - c4-pre-sort

2024-04-28T05:56:22Z

JustDravee marked the issue as duplicate of #67

#1 - c4-pre-sort

2024-04-29T09:06:17Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-08T11:50:07Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-08T12:51:16Z

koolexcrypto marked the issue as satisfactory

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter