DYAD - 0xnev's results

Lines of code

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

Vulnerability details

Impact

There is an explicit invariant within the DYAD system, that is

At least $1 of non-Kerosene collateral per DYAD minted in their Note upon the successful execution of the mint or withdrawal transaction.

This check is implemented as per the following within withdrawals:

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

The issue here is, during withdrawals from unbounded kerosene vaults, the value of kerosene is also included for the exogenous collateral sufficiency check.

The result is that even if there are sufficient exogenous collateral to meet minimum collateral ratio after withdrawal, the withdrawal will revert, potentially blocking withdrawals of kerosene unintentionally.

Proof of Concept

1. Bob has 150_000 USD worth of collateral minted against 50_000 USD worth of DYAD, assuming 55_000 USD worth of collateral comes from kerosene value

<br/>

2. Bob wants to withdraw 55_000 USD worth of kerosene, so 95_000 - 55_000 = 40000 < 50_000, reverts

<br/>

3. You can see from above that after the withdrawal, Bob's collateral ratio would have still been healthy at 95_000 / 50_000 = 1.9, and exogenous collateral check would have still been sufficient (100_000 USD to 50_000 DYAD minted)

Tools Used

Manual Analysis

Recommended Mitigation Steps

If kerosene withdrawals are intiated, set value to zero, given the collateral ratio check would be sufficient in ensuring healthy CR ratio.

  function withdraw(
    uint    id,
    address vault,
    uint    amount,
    address to,
+   bool    withdrawKerosene
  ) 
    public
      isDNftOwner(id)
  {
    if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();
    uint dyadMinted = dyad.mintedDyad(address(this), id);
    Vault _vault = Vault(vault);
+   uint value;
+   if (withdrawKerosene) {
+       value = 0;
+   } else {
+       value = amount * _vault.assetPrice() 
+                     * 1e18 
+                     / 10**_vault.oracle().decimals() 
+                     / 10**_vault.asset().decimals();
+   }

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

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Notice in the liquidate() function, collateral is only moved from liquidatee non-kerosene vaults (in vaults mapping) to the liquidator note balance.

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

In the previous vault manager, kerosene vaults were not licensed as collateral, so a CDP always start with at least CR of 150% non-kerosene collateral.

Now, CDP can start with at least 100% non-kerosene collateral + 50% kerosene value.

This can result in a lack of incentivization to liquidate specific CDPs backed by kerosene collateral even though liquidation bonus is present, leading to possible bad debt.

This issue can also be abused during volatile markets where price of collateral decreases, where users can front-run chainlink price updates and withdraw collateral for the price gap. This could also be exacerbated by depositing into bounded kerosene vaults where price is always 2x that of unbounded kerosene allowing users to maintain undercollaterized positions by abusing value of kerosene maintained.

Proof of Concept

We can make a comparision

VaultManagerV1: Assume 1 WETH = $2000 USD

• Assume there is a position of 10_000 DYAD minted against 7 WETH (14_000 USD) of collateral, CR = 1.4
• Liquidator liquidates, burning 10_000 DYAD and receiving, (40% * 20% + 100%) / 140% * 7 = 5.4 WETH worth of collateral
• This liquidation would be profitable given burning 10000 DYAD would return 5.4 * 2000 = 10800 USD worth of collateral, profitting 800 USD

VaultManagerV2:

• Assume there is a position of 10_000 DYAD minted against 10_000 USD collateral (5 WETH) from non kerosene collateral and 4000 USD from kerosene collateral (Price can be manipulated as noted in my other submission, depends on other factors as well)
• Liquidator liquidates, burning 10_000 DYAD and earning, (40% * 20% + 100%) / 140% * 5 = ~3.857 WETH
• This liquidation would not be profitable given burning 10000 DYAD would return 3.857 * 2000 = ~7714 USD worth of collateral, losing 2286 USD

Tools Used

Manual Analysis

Recommended Mitigation Steps

Consider moving collateral from kerosene vaults as well stored within vaultsKerosene

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The maximum number of non-kerosene/kerosene vaults allowed to be added via remove()/removeKerosene() is currently set as 5 represented by MAX_VAULTS/MAX_VAULTS_KEROSENE. To remove a vault, the remaining asset in the vault must be zero as presented by the following check here and here

if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();

Notice that depositing is permisionless, that is any user can deposit in place of other users to via deposit() as long as NOTEs

While it is a good sanity check, in the future when multiple riskier assets/kerosene vaults are available (> 5 types of assets), where collateral price are more volatile, this could pose a problem if a user simply front-runs a remove call by depositing just 1 wei of asset into the vault for the user.

Take an example of a user having the maximum number of non-kerosene vaults (5) added, he wants to derisk from a volatile collateral (e.g. WETH) by redeeming all collateral and switching to a more stable vault such as a stablecoin vault (e.g. DAI, USDC etc ...), but cannot do so. If collateral prices keep falling, then the user either risk getting liquidated or is forced to burn additional DYAD to prevent getting liquidated instead of having the ability to switch vaults and redeem some collateral beforehand.

Tool Used

Manual Analysis

Recommendation Mitigation Steps

• Only allow deposits by owner of NFT i.e. use isDNftOwner modifier
• Implement a minimum deposit amount to increase cost of donation.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

The valuation of kerosene is calculated as:

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

whereby:

$K_{\text{value}} $ = Deterministic value of Kerosene $TVL$ = Total USD value of collateral in non-kerosene and kerosene vaults $D_{\text{supply}}$ = Total supply of DYAD minted ${K_{\text{supply}}}$ = Circulating supply of kerosene i.e kerosene distributed

Large liquidity providers can manipulate the deterministic value of kerosene by depositing large amounts of non-kerosene/kerosene collateral and withhold minting DYAD. This inflates the deterministic value of K.

These large LPs can possibly profit by targeting users that provides kerosene as collateral for DYAD by the following steps

1. deposit() - Deposit large amounts of collateral into non-kerosene vaults to inflate TVL, but withhold minting DYAD. This inflates K value

<br/>

2. Wait for some users to open new CDP positions specifically those that provides kerosene as collateral with current value of Kerosene

<br/>

3. withdraw() - Withdraw the large liquidity provided. This results in K value dropping sharply

<br/>

4. liquidate() - Target and liquidate users that provides kerosene as primary collateral for CDP using another account holding DYAD minted

Executing a large deposits withdrawals and subsequently liquidate other users since kerosene prices can drop sharply, which consequently sharply decreases collateral ratio of users using kerosene as collateral. This means non-kerosene depositors can potentially force liquidations on kerosene depositors using two separate accounts.

Proof of Concept

Example:

1. Assumptions $TVL$ = 3 million USD $D_{\text{supply}}$ = 1 million Overall CR = 3 ${K_{\text{supply}}}$ = 100 million $ K_{\text{value}}$ = 3 million - 1 million / 100 million = $0.02 USD

<br/>

2. Assume attacker deposits 6 million USD worth of collateral $ K_{\text{value}}$ = 9 million - 1 million / 100 million = $0.08 USD

<br/>

3. Assume a user mints DYAD using kerosene as collateral, mints 10000 DYAD against 0.125 million kerosene and 10_000 USD worth of collateral (20_000 USD), User CR = (20_000 / 10_000) = 2

<br/>

4. Attacker withdraws 6 million USD worth of collateral $ K_{\text{value}}$ = 3 million - 1.01 million / 100 million = $0.0199 USD User CR = (0.0199 * 0.25 million kerosene) + 10000 / 10_000 = 1.4975

<br/>

5. Attacker calls liquidate on user using surplus DYAD minted from non-kerosene vault, burning 10_000 DYAD and gaining ~73.4% of the users non-kerosene collateral 0.734 * 10000 = 7340 $ K_{\text{value}}$ = 3 million - 1 million / 100 million = $0.02 USD

<br/>

6. Large LP deposits again 6 million USD worth of collateral, inflating price of kerosene back to 0.08 USD. This allows him to deposit and mint additional DYAD, profiting 7340 USD collateral + a portion DYAD minted used for liquidation is reminted (e,g 3670), essentially meaning the attack would only involve the cost of gas at the expense of a large portion of the users collateral $ K_{\text{value}}$ = 3 million - 1 million / 100 million = $0.02 USD

<br/>

7. Large LP repeats attack targeting users that provides kerosene as collateral to mint DYAD

This issue is possibly compounded because any existing users that provided kerosene as collateral could also now mint additional DYAD (as long as DYAD is backed 1:1 by non-kerosene collateral), whereby if they do so, $ K_{\text{value}}$ would drop further due to increase in $D_{\text{supply}}$. They are exposed to higher risk of getting liquidated as well as they are now exposed to a higher CDP based on previously inflated kerosene value.

Then when the large liquidity withdrawal is executed, liquidation might be even more profitable since $ K_{\text{value}}$ would drop even more.

The only external condition here is the CDP must partially be backed by kerosene as collateral.

Tools Used

Manual Analysis

Recommended Mitigation Steps

I believe the fix is non-trivial, and valuation of K could be required to be refactored.

• Refactor how Kvalue is being determined, such as only including non-kerosene TVL that has minted DYAD position
• Enforce minting of DYAD for users depositing

Assessed type

Other