DYAD - dimulski's results

Lines of code

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

Vulnerability details

Impact

The DYAD protocol is requiring a collateral ratio of 150% for minting DYAD tokens. Each DYAD stablecoin should be backed by at least $1.50 of exogenous collateral. This surplus is intended to absorb the collateral’s volatility, in order to keep DYAD fully backed in all conditions. However the collateral ratio restrictions are easily bypassed because of the way the collateral ratio is calculated. As per the deploy script the same wstETH and WETH backed vaults will be utilized in the getNonKeroseneValue() and getKeroseneValue() functions (the user have to first add them to the lists of his approved vaults). Also this is the way the protocol have decided to calculate the value of the Kerosene token. As per the docs: Kerosene is a token that lets you mint DYAD against this collateral surplus. Kerosene can be deposited in Notes just like other collateral to increase the Note’s DYAD minting capacity. However a user can just add the vaults to both the vaults and vaultsKerosene lists. And thus doubling his collateral ratio, bypassing the protocol collateral ratio restrictions, and minting dyad tokens at 1:1 ratio with the deposited collateral. If the dollar value of the collateral decreases slightly, the protocol will go underwater easily if enough such positions exists. Also when it comes to liquidation, the collateral ratio check will be reverting until the provided collateral is well below the dollar value of the minted dyad.

Proof of Concept

Gist

After following the steps in the above linked gist add the following test to AuditorTests.t.sol file:

    function test_DoubleCollateral() public {
        vm.startPrank(alice);
        WETH.mint(alice, 10e18);
        console2.log("Balance of weth vault: ", WETH.balanceOf(address(vault)));
        dNft.mintNft(alice);
        console2.log("Owner of 0 id NFT: ", dNft.ownerOf(0));
        vaultManagerV2.add(0, address(vault));
        vaultManagerV2.add(0, address(vaultWstEth));
        vaultManagerV2.add(0, address(unboundedKerosineVault));
        WETH.approve(address(vaultManagerV2), type(uint256).max);
        vaultManagerV2.deposit(0, address(vault), 1e18);
        oracleWETH.setPrice(int256(150e8));
        vaultManagerV2.mintDyad(0, 100e18, alice);
        console2.log("Alice's note collateral ratio: ", vaultManagerV2.collatRatio(0));
        vaultManagerV2.addKerosene(0, address(vault));
        vaultManagerV2.addKerosene(0, address(vaultWstEth));
        console2.log("Alice's note collateral ratio after she adds the vaults for calculating the kerosene value: ", vaultManagerV2.collatRatio(0));
        vm.stopPrank();
    }

Logs:
  Balance of weth vault:  0
  Owner of 0 id NFT:  0x328809Bc894f92807417D2dAD6b7C998c1aFdac6
  Alice's note collateral ratio:  1500000000000000000
  Alice's note collateral ratio after she adds the vaults for calculating the kerosene value:  3000000000000000000

To run the test use: forge test -vvv --mt test_DoubleCollateral

Tools Used

Manual review and Foundry

Recommended Mitigation Steps

Consider implementing internal accounting to track the price of kerosine, don't add vaults, and definitely don't add the same vaults.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

There are no restrictions on the maximum collateral amount a user can deposit into a single Note, and the amount of DYAD tokens he can mint. The liquidate() function requires all of the DYAD tokens, a Note has minted to be burned in a single transaction as can be seen from this line:

dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));

Typically liquidators are bots that monitor protocols and liquidate undercollateralized positions if there is profit to be made. They don't hold the specific funds that will be required to be transferred to the protocol, or burned in order to execute a potentially profitable liquidation. They rely on flash loans, and most protocols providing flash loan functionality, require users to pay back the loan in the same token they received when taking the flash loan + some fee, paid in the same token. There are no DYAD markets on AAVE or Compound, so a liquidator can't take a big enough loan of DYAD tokens in order to liquidate an undercollateralized position. Thus the liquidator will have to acquire the DYAD tokens required to be burned in a different way. At the time of writing this report there are no DYAD pools on Curve. There is a Uniswap V2 ETH/DYAD pool which contains 31714921551045792676107 ≈ 31_714e18 DYAD tokens, as well as Uniswap V3 USDC/DYAD pool which contains ≈ 336_153e18 DYAD tokens. There are around 370_000e18 DYAD tokens supplied as liquidity on DEXes. However the slippage on the Uniswap V2 pool will be extremely big. As for Uniswap v3 pools, tokens are converted to the other token, when the current price, is not in the price range the liquidity was provided for. So we can assume that a liquidator can make a swap of 150_000 - 200_000 DYAD tokens max. To create a position of $150_000 - $200_000 DYAD tokens with a collateralization rate of 150%, a user would have to provide $225_000 - $300_000 worth of collateral, to satisfy the collateral ratio requirement. The deposti() function allows for anyone to provide collateral for a note, and the mintDyad() function allows the owner of the Note to mint DYAD tokens to an arbitrary address. Many users not wanting to buy a Note NFT, which price starts at 0.1e18 ETH and increase linearly on each new minted NFT with 0.001e18ETH, may opt in for a single Note, and provide collateral in it, thus creating one big position, split between different users. Due to the implementation of the liquidate() function all of the DYAD of the undercollateralized position have to be burned in one transaction. It will take 1 or 2 big positions, in the range of 150_000 - 200_000 DYAD tokens, to go below the collateralization ratio, and at the time of writing this report there isn't a way to get this positions liquidated. The whole protocol can go underwater very quickly.

Tools Used

Manual review

Recommended Mitigation Steps

Either provide a maximum amount of DYAD tokens that a single Note can mint, or allow undercollateralized positions to be liquidated on parts.

Assessed type

Context

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#L119-L131

Vulnerability details

Impact

The withdraw() function in implements the following check:

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

not allowing withdraws in the same block, in which someone has already deposited into a so called Note. As can be seen from the deposit() function:

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

There are no minimum deposits required, so a malicious actor can frontrun the Note owner call to withdraw(), deposit as little as 1 WEI and the withdraw function will revert, thus griefing the Note owner. A malicious actor can do this as many times as he wants, and the Note owner won't be able to withdraw his deposited tokens. In a turbulent market conditions (which are a typical occurrence in crypto), a Note owner may have decided that he would like to convert his crypto assets to fiat currency, already burned all of his minted DYAD tokens in order to free all of his collateral. However as he is trying to withdraw all of his deposited collateral, a malicious actor can grief him as described above, while the deposited collateral falls in value. Thus resulting in immense losses for the owner of the Note. The same griefing attack can be performed when a user is trying to remove a vault from his vaults, or vaultsKerosene lists. For example he has liquidated an undercollateralized position of somebody, he already has 5 vaults in his vaults list but a big portion of the collateral he received as a reward are in a vault he hasn't added to his vaults list. In order to withdrawn them he will have to remove one vault and add another, however a malicious user can keep depositing 1 WEI, thus restricting this functionality. Considering when a positions gets liquidated, it is typically because the collateral asset fell in dollar value, and is probably going to continue falling in price due to some turbulent market conditions, this may result in a big loss for users.

Tools Used

Manual review

Recommended Mitigation Steps

Consider allowing only the DNFT Owner to be able to deposit assets via the deposit() function, set the modifier from isValidDNft(id) to isDNftOwner(id)

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

The DYAD protocol supports two different Kerosine vaults - bounded and unbounded. Users who deposit into the unbounded vault should be able to withdraw their kerosene tokens if the collateral ratio criteria are met. Withdrawing of deposited tokens happens via the withdraw() function.

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

As can be seen from the code snippet above when the value variable is calculated the contract tries to call oracle().decimals() on the corresponding vault, however there is no oracle variable in the Vault.kerosine.unbounded.sol contract and the transaction will revert, locking the deposited kerosine tokens forever.

Proof of Concept

Gist After following the steps in the above linked gist add the following test to AuditorTests.t.sol:

    function test_WithdrawKerosineNotWorking() public {
        vm.startPrank(owner);
        console2.log("Kerosene balance of owner normalized: ", kerosine.balanceOf(owner) / 1e18);
        kerosine.transfer(alice, 10_000e18);
        console2.log("Kerosene balance of alice normalized: ", kerosine.balanceOf(alice) / 1e18);
        vm.stopPrank();

        vm.startPrank(alice);
        WETH.mint(alice, 10e18);
        deal(alice, 1e18);
        console2.log("Balance of weth vault: ", WETH.balanceOf(address(vault)));
        dNft.mintNft(alice);
        console2.log("Owner of 0 id NFT: ", dNft.ownerOf(0));
        vaultManagerV2.add(0, address(vault));
        vaultManagerV2.add(0, address(vaultWstEth));
        vaultManagerV2.add(0, address(unboundedKerosineVault));
        vaultManagerV2.addKerosene(0, address(vault));
        vaultManagerV2.addKerosene(0, address(vaultWstEth));
        console2.log("Alice deposited kerosine in the unbounded vault: ", unboundedKerosineVault.id2asset(0));
        kerosine.approve(address(vaultManagerV2), type(uint256).max);
        vaultManagerV2.deposit(0, address(unboundedKerosineVault), 10_000e18);
        console2.log("Alice deposited kerosine in the unbounded vault normalized : ", unboundedKerosineVault.id2asset(0) / 1e18);
        console2.log("Alice note minted dyad: ", dyad.mintedDyad(address(vaultManagerV2), 0));
        vm.roll(100);
        vm.expectRevert();
        vaultManagerV2.withdraw(0, address(unboundedKerosineVault), 10_000e18, alice);
        vm.stopPrank();
    }

Logs:
  Kerosene balance of owner normalized:  1000000000
  Kerosene balance of alice normalized:  10000
  Balance of weth vault:  0
  Owner of 0 id NFT:  0x328809Bc894f92807417D2dAD6b7C998c1aFdac6
  Alice deposited kerosine in the unbounded vault:  0
  Alice deposited kerosine in the unbounded vault normalized :  10000
  Alice note minted dyad:  0

To run the test use: forge test -vvv --mt test_WithdrawKerosineNotWorking

Tools Used

Manual review & Foundry

Recommended Mitigation Steps

Consider adding some identifier to the unbounded kerosine vault, and implement a separate withdraw function, or in the current withdraw function check if the vault the user is trying to withdraw from is a kerosine vault, and if so calculate the price accordingly. For example:

if(vaultKerosineUnbounded) {
   value = amount * _vault.assetPrice() / 1e8;
}

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

As per the readme of the contest, the Deploy.V2.s.sol contract is in scope, and in this file the steps to migrate to the new VaultManagerV2.sol are defined. However the protocol team has neglected the fact that there is already a circulating supply of DYAD tokens. As of writing the report the total supply is: 632_967_400_000_000_000_000_000 ≈ 632_967e18. And when the Unbounded Kerosine Vault is deployed the address of the already existing DYAD token is set.

    UnboundedKerosineVault unboundedKerosineVault = new UnboundedKerosineVault(
      vaultManager,
      Kerosine(MAINNET_KEROSENE), 
      Dyad(MAINNET_DYAD),
      kerosineManager
    );

In Vault.kerosine.unbounded.sol the asset price is calculated in the following way:

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

As we can see the DYAD token total supply is subtracted from the tvl, and later on some addition calculations are performed. However in the deployment script new wethVault and wstETHVault are created with 0 funds, and added to the kerosineManager. When a function calls the assetPrice() function it will revert as the TVL will initially be 0, and the DYAD token supply is approximately 632_967e18. Functions like withdraw(), liquidate(), redeemDyad(), mintDyad() will always revert if the Note owner, has added the unbounded kerosine vault to his list of vaults, no matter if he has deposited funds in it or not. It will require at least $632_967 worth of collateral to be deposited to the vaults, before any new DYAD can be minted.

Tools Used

Manual review

Recommended Mitigation Steps

Use the already deployed vaults, where the collateral is provided

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

The liquidation mechanism is not optimal leading to a couple of detrimental issues for the protocol. In order for liquidation to be executed on time, so the protocol is always well collateralized(above 150%) and the chances of going underwater are slim, usually bots are the ones who look for potential profit when executing liquidations on different protocols. However the design of the liquidate() function, will deter bots of liquidating, medium positions. Possible only very big positions will be profitable enough for bots >$20_000

  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 can be seen from the above code snippet, in order for an undercollateralized position to be liquidated, the liquidator have to have a Note. The price to mint a Note or a DNft is calculated in the following way:

uint price = START_PRICE + (PRICE_INCREASE * publicMints++);

The START_PRICE is 100000000000000000 ≈ 0.1ETH and the PRICE_INCREASE is 1000000000000000 ≈ 0.001ETH, this parameters are immutable. At the time of writing this report there are 531 publicMints. Meaning that in order for a bot to be able to liquidate a transaction he will first have to mint a Note costing at least 0.1 + (0.001 * 531) = 0.631ETH transaction costs excluded. This is worth around $2000. So the 20% liquidation bonus that the liquidator received should amount to at least $2000 (not including gas, swapping fees, and slippage). Most liquidation bots, hold their assets in stable coins such as USDC and USDC, and swap to the required collateral, liquidate the position, and then swap back to a stable asset. However there is another problem with the liquidate() function. Bounded Kerosine can't be withdrawn from a vault, so liquidators will be stuck with an asset that they can't withdraw and convert to a stable asset. Assuming more that 10% - 15% of the undercollateralized position is in bounded kerosine, this will further deter liquidation bots. This is extremely bad for the protocol, and the possibility of the whole protocol going underwater in a slightly turbulent market is huge. It is true that undercollateralized positions can be liquidated by other normal participants in the DYAD ecosystem, however this won't happen fast enough, and still people who don't want to hold unbounded kerosine, will receive it as part of the liquidation reward. According to Dune, the 8th biggest position is worth 25_776e18 DYAD tokens, the 9th is 13_000e18 DYAD tokens, this means more than 90% of all positions most probably won't be liquidated on time, which can drive the protocol underwater extremely fast.

Tools Used

Manual review

Recommended Mitigation Steps

Consider a way to directly send the collateral to the address of the liquidator, don't require a Note. Dealing with not sending bounded kerosine to the liquidator is trickier. Consider the implementation of a mechanism that will allow people to swap it directly for some asset such as DYAD for example.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

The DYAD protocol allows users to deposit as little as 1 WEI via the deposit() function, however in order to mint the DYAD token the protocol requires user to have a collateral ratio of 150% or above. Liquidators liquidate users for the profit they can make. Currently the DYAD protocol awards the value of the DYAD token burned(1 DYAD token is always equal to 1$ when calculated in the liquidate function) + 20% of the collateral left to the liquidator.

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

If there is no profit to be made than there will be no one to call the liquidate function. Consider the following example:

• User A deposits collateral worth 75$, and mint 50 DYAD tokens equal to 50$. The collateral ratio is 75/50 = 150%.
• The price of the provided collateral drops, and now user A collateral is worth 70$, 70/50 = 140% collateral ratio. The position should be liquidated now, so the protocol doesn't become insolvent.
• We get the following calculation:
  • cr & cappedCr = 1.4e18
  • liquidationEquityShare = (1.4e18 - 1e18) * 0.2e18 = 80000000000000000 = 0.08e18
  • liquidationAssetShare = (0.08e18 + 1e18) / 1.4e18 = 771428571428571428 ≈ 0.77e18
• The total amount of collateral the liquidator will receive is 70 * 0.77 = 53.9$
• The dollar amount he spent for the DYAD token is 50$ (assuming no depegs) so the profit for the liquidator will be 53.9$ - 50$ = 3.9$

The protocol will be deployed on Ethereum where gas is expensive. Because the reward the liquidator will receive is low, after gas costs taking into account that most liquidators are bots, and they will have to acquire the DYAD token on an exchange, experience some slippage, swapping fees, and additional gas cost, the total cost to liquidate small positions outweighs the potential profit of liquidators. In the end these low value accounts will never get liquidated, leaving the protocol with bad debt and can even cause the protocol to go underwater. Depending on the gas prices at the time of liquidation (liquidity at DEXes can also be taken into account, as less liquidity leads to bigger slippage) positions in the range of 150$ - 200$ can be unprofitable for liquidators. This attack can be beneficial to a whale, large competitor, or a group of people actively working together to bring down the stable coin. The incentive for them is there if they have shorted the DYAD token with substantial amount of money. The short gains will outweigh the losses they incur by opening said positions to grief the protocol. Also this is crypto, there have been numerous instances of prices drooping rapidly, and usually at that time gas prices are much higher compared to normal market conditions. NOTE: The liquidation mechanism is extremely inefficient, as it requires bots to have a Note in order to be able to liquidate a position, however this is a separate issue, as even the inefficiency of the liquidation mechanism is fixed, small positions still won't be profitable enough for liquidators.

Tools Used

Manual review

Recommended Mitigation Steps

Consider setting a minimum amount that users have to deposit before they can mint DYAD stable coin. Minimum amount of 500-600$ should be good enough.

Assessed type

Context

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#L119-L131

Vulnerability details

Impact

The withdraw() function in implements the following check:

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

not allowing withdraws in the same block, in which someone has already deposited into a so called Note. As can be seen from the deposit() function:

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

There are no minimum deposits required, so a malicious actor can frontrun the Note owner call to withdraw(), deposit as little as 1 WEI and the withdraw function will revert, thus griefing the Note owner. A malicious actor can do this as many times as he wants, and the Note owner won't be able to withdraw his deposited tokens. In a turbulent market conditions (which are a typical occurrence in crypto), a Note owner may have decided that he would like to convert his crypto assets to fiat currency, already burned all of his minted DYAD tokens in order to free all of his collateral. However as he is trying to withdraw all of his deposited collateral, a malicious actor can grief him as described above, while the deposited collateral falls in value. Thus resulting in immense losses for the owner of the Note. The same griefing attack can be performed when a user is trying to remove a vault from his vaults, or vaultsKerosene lists. For example he has liquidated an undercollateralized position of somebody, he already has 5 vaults in his vaults list but a big portion of the collateral he received as a reward are in a vault he hasn't added to his vaults list. In order to withdrawn them he will have to remove one vault and add another, however a malicious user can keep depositing 1 WEI, thus restricting this functionality. Considering when a positions gets liquidated, it is typically because the collateral asset fell in dollar value, and is probably going to continue falling in price due to some turbulent market conditions, this may result in a big loss for users.

Tools Used

Manual review

Recommended Mitigation Steps

Consider allowing only the DNFT Owner to be able to deposit assets via the deposit() function, set the modifier from isValidDNft(id) to isDNftOwner(id)

Assessed type

DoS