DYAD - eta's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/KerosineManager.sol#L24-L25 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/KerosineManager.sol#L50

Vulnerability details

Impact

Although boundedKerosineVault not being initially licensed in the Deploy.V2.s.sol script, it still could be added(isLicensed) under certain conditions in KerosineManager.sol contract. This can lead to liquidation failure due to the condition collatRatio(id) >= MIN_COLLATERIZATION_RATIO revert in the VaultManagerV2.sol contract.

Proof of Concept

During the deployment of the Deploy.V2.s.sol script, the boundedKerosineVault contract was not added to the vaultLicenser, resulting in isLicensed[vault] being false for the boundedKerosineVault contract.

Deploy.V2.s.sol#L96

    // vaultLicenser.add(address(boundedKerosineVault));

However, in the KerosineManager.sol contract, the boundedKerosineVault contract can be added as long as two conditions are met: if (vaults.length() >= MAX_VAULTS) revert TooManyVaults(); and if (!vaults.add(vault)) revert VaultAlreadyAdded();, which means that the isLicensed status of this boundedKerosineVault contract becomes true.

KerosineManager::add#L24-25

  function add(
    address vault
  ) 
    external 
      onlyOwner
  {
if (vaults.length() >= MAX_VAULTS) revert TooManyVaults();
// @audit the boundedKerosineVault could be added    
if (!vaults.add(vault))            revert VaultAlreadyAdded();
  }

KerosineManager::isLicensed#L50

  function isLicensed(
    address vault
  ) 
    external 
    view 
returns (bool) {
// @audit the boundedKerosineVault could be isLicensed
      return vaults.contains(vault);
  }

Next, let's examine the boundedKerosineVault contract, which can adjust the price of Kerosine by only deposits without withdrawals and its assetPrice() * 2. It's crucial to remember this point.

Vault.kerosine.bounded::withdraw#L41

  function withdraw(
    uint    id,
    address to,
    uint    amount
  ) 
    external 
    view
      onlyVaultManager
  {
// @audit only deposits without withdrawals
    revert NotWithdrawable(id, to, amount);
  }

Vault.kerosine.bounded::assetPrice#L49

  function assetPrice() 
    public 
    view 
    override
returns (uint) {
// @audit assetPrice() * 2
      return unboundedKerosineVault.assetPrice() * 2;
  }

The key arises in the liquidate function of the VaultManagerV2.sol contract. It checks if collatRatio(id) >= MIN_COLLATERIZATION_RATIO, which could revert. let's take a look at how the collatRatio() function is calculated. It first calls getTotalUsdValue(), which in turn calls the getKeroseneValue() function. The getKeroseneValue() function calculates the usdValue of all isLicensed assets by calling getUsdValue() and then assetPrice(), including the previously added boundedKerosineVault contract.

VaultManagerV2::liquidate#L213-214

  function liquidate(
    uint id,
    uint to
  ) 
    external 
      isValidDNft(id)
      isValidDNft(to)
    {
      uint cr = collatRatio(id);
     // @audit it could revert
      if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();

VaultManagerV2::collatRatio#L238

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

VaultManagerV2::getTotalUsdValue#L247

  function getTotalUsdValue(
    uint id
  ) 
    public 
    view
returns (uint) {
// @audit it calls getKeroseneValue()
      return getNonKeroseneValue(id) + getKeroseneValue(id);
  }

VaultManagerV2::getKeroseneValue#L281

  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))) {
// @audit it calls getUsdValue() and then assetPrice() to get usdValue
          usdValue = vault.getUsdValue(id);        
        }
        totalUsdValue += usdValue;
      }
      return totalUsdValue;
  }

The problem occurs when we add the boundedKerosineVault contract(or to make it isLicensed). Its value increases the return value of getKeroseneValue(), which in turn increases the return value of getTotalUsdValue(), ultimately leading to an increase in the return value of collatRatio(). As a result, the condition collatRatio(id) >= MIN_COLLATERIZATION_RATIO reverts, causing the liquidate() function to fail.

Recommendation:

Consider whether the calculation of collatRatio() should include the isLicensed boundedKerosineVault or if the isLicensed boundedKerosineVault should still serve as a functional role for the boundedKerosineVault, or if it should be upgraded to an unboundedKerosineVault.

Assessed type

Context