DYAD - Topmark's results

Lines of code

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

Vulnerability details

Impact

• User can avoid Liquidation by breaking validation at L213-L214 of the VaultManagerV2 contract
• User can withdraw even when Collateral Ratio should be below the Minimum collateral Ratio allowed at L152 of the VaultManagerV2 contract
• User can mint Dyad when Collateral Ratio should be below the Minimum collateral Ratio allowed at L167 of the VaultManagerV2 contract

Proof of Concept

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

The two functions above from the VaultManagerV2 contract shows how callateral ratio is calculated, it calls getTotalUsdValue(...) which returns the sum of NonKerosene USD Value and Kerosene USD Value.

  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 two functions above shows the implementation to calculate the non kerosene and Kerosene value, as noted from the pointers the problem is that the usdValue is derived exactly the same way for both functions by calling vault.getUsdValue(id) . The implication of this is that when the id has both the license of the keroseneManager and license from the vaultLicenser, the same USD value of the ID would be calculated twice and doubled in the getTotalUsdValue(...) function thereby inflating collateral value and ratio

Tools Used

Manual Review

Recommended Mitigation Steps

Protocol should put necessary implementation to ensure a vault cannot have both Vault License and Kerosene License, another way is to add different implementations to calculate these different USD values

Assessed type

Access Control