DYAD - KYP's results

Lines of code

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

Vulnerability details

Relevant code:VaultManagerV2::liquidate

Description

When liquidating a DYAD position works in the following way:

1. Liquidator brings in the necessary DYAD to cover for liquidatee's minted DYAD in full.
2. Liquidator then acquires a proportional share of the liquidatee's collateral.

The issue occurs when the biggest DYAD minter's CR falls below MIN_COLLATERIZATION_RATIO. Few things happen then:

• Someone has to mint more collateral, and then liquidate them - which could take time.
• When the liquidatee has minted too much DYAD for any other user to mint and cover for them.
• In extreme cases CR could fall below 100%, removing the incentive to liquidate.

There is a scenario in bear markets where collateral might tank and then liquidations should start to occur. In this case, if liquidations are delayed, the total value of collateral assets might end up less than the circulating DYAD.

Root Cause

Lack of partial liquidations

Impact

In the event of collateral depreciation, the protocol might not be able to liquidate positions fast enough. In the worst case this would cause DYAD depeg.

PoC

// SPDX-License-Identifier: MIT
pragma solidity =0.8.17;

import "forge-std/console.sol";
import "./VaultManagerHelper.t.sol";
import "../src/core/Vault.wsteth.sol";
import "../src/core/Vault.kerosine.unbounded.sol";
import "../src/staking/Kerosine.sol";
import "../src/core/VaultManagerV2.sol";
import {OracleMock} from "./OracleMock.sol";
import {IAggregatorV3} from "../src/interfaces/IAggregatorV3.sol";

contract VM2Liquidation is VaultManagerTestHelper {
  address whale = makeAddr("whale");
  address user = makeAddr("user");

  VaultManagerV2 vm2;

  function test_impossible_liquidation() public {
    setupContracts();

    // Consider whale is the user with the largest collateral in the system
    uint256 whaleCollateral = 170_000 ether; // collateral of a token

    // Setup whale dNFT position
    startHoax(whale, 1 ether);
    uint whaleDNFT = dNft.mintNft{value: 1 ether}(whale);
    vm2.add(whaleDNFT, address(wethVault));

    deal(address(weth), whale, whaleCollateral);
    weth.approve(address(vm2), whaleCollateral);
    vm2.deposit(whaleDNFT, address(wethVault), whaleCollateral);

    console.log("Value of whales token deposit: ", vm2.getNonKeroseneValue(whaleDNFT));

    // mint DYAD with 150% CR; 1 token == 1000 usd
    vm2.mintDyad(whaleDNFT, 113_333_333 ether, whale);
    console.log("DYAD held by whale: ", dyad.balanceOf(whale));

    // Setup user dNFT position
    uint256 userCollateral = 80_000e8;

    startHoax(user, 1 ether);
    uint userDNFT = dNft.mintNft{value: 1 ether}(user);
    vm2.add(userDNFT, address(daiVault));

    deal(address(dai), user, userCollateral);
    dai.approve(address(vm2), userCollateral);
    vm2.deposit(userDNFT, address(daiVault), userCollateral);

    // 1 dai token == 1 usd
    console.log("Value of dai tokens: ", vm2.getNonKeroseneValue(userDNFT));

    // mint DYAD with 150% CR ~ 53K DYAD
    vm2.mintDyad(userDNFT, 53333_33333333, user); 
    console.log("DYAD held by user: ", dyad.balanceOf(user));

    // Now, the value of the whale's collateral declines
    wethOracle.setPrice(650e8); // price falls 30%

    console.log("New Value of whales token deposit: ", vm2.getNonKeroseneValue(whaleDNFT));
    console.log("Whale CR: ", vm2.collatRatio(whaleDNFT));

    // Liquidation can occur only if some user mints ~133K DYAD - but this is not always possible
    // - The market would not always want collateral that just fell 30% in value
    // - A single user might not have additional collateral
    vm.expectRevert();
    vm2.liquidate(whaleDNFT, userDNFT);
  }

  function setupContracts() internal {
    dNft       = new DNft();
    weth       = new ERC20Mock("WETH-TEST", "WETHT");
    wethOracle = new OracleMock(1000e8);

    vaultManagerLicenser = new Licenser();
    vaultLicenser        = new Licenser();

    dyad = new Dyad(vaultManagerLicenser);

    vm2 = new VaultManagerV2(
      dNft,
      dyad,
      vaultLicenser
    );
    vaultManagerLicenser.add(address(vm2));

    wethVault = new Vault(
      vm2,
      ERC20(weth),
      IAggregatorV3(address(wethOracle))
    );

    daiOracle = new OracleMock(1e8); // init price with 8 decimals, because OracleMock::decimals() isn't configurable 
    daiVault = new Vault(
      vm2,
      ERC20(dai),
      IAggregatorV3(address(daiOracle))
    );

    Kerosine kerosine = new Kerosine();

    KerosineManager kerosineManager = new KerosineManager();

    kerosineManager.add(address(wethVault));
    kerosineManager.add(address(daiVault));

    vm2.setKeroseneManager(kerosineManager);

    UnboundedKerosineVault unboundedKerosineVault = new UnboundedKerosineVault(
      vaultManager,
      kerosine,
      dyad,
      kerosineManager
    );

    BoundedKerosineVault boundedKerosineVault     = new BoundedKerosineVault(
      vaultManager,
      kerosine,
      kerosineManager
    );

    KerosineDenominator kerosineDenominator       = new KerosineDenominator(
      kerosine
    );

    unboundedKerosineVault.setDenominator(kerosineDenominator);

    unboundedKerosineVault.transferOwnership(address(this));
    boundedKerosineVault.  transferOwnership(address(this));

    vaultLicenser.add(address(daiVault));
    vaultLicenser.add(address(wethVault));
    vaultLicenser.add(address(unboundedKerosineVault));

    vaultLicenser.transferOwnership(address(this));
  }

}

Suggested Mitigation

Allow for partial liquidations

Assessed type

Other

Lines of code

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

Vulnerability details

VaultManagerV2::remove VaultManagerV2::deposit

Description

In the protocol each dNFT can have up to 5 asset vault added to it, to be used as collateral. Each vault contains a separate asset. In order for the owner to remove a vault, he needs to NOT have any assets in it. However anyone can call the deposit function to any vault. A malicious actor can DoS an NFT owner from removing a vault from his NFT by sending dust amounts of asset. In the worst case scenario a user can NOT add a collateral vault and improve his CR.

Root Cause

Access control

Impact

DoS

Suggested Mitigation

Let only the NFT owner deposit assets

  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

Access Control