DYAD - Josh4324's results

Lines of code

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

Vulnerability details

Impact

Users are unable to withdraw their kerosine token deposited into their notes, the vault manager is compatible with several vaults such as weth vault and the wsteth vault. The implementation of the withdraw function in the vault manager does not take into account that the kerosine vault does not have an oracle function like other non kerosine vaults.

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

Line 146 to 149 of the withdraw function which is pointed above shows that the withdrawal function tries to call the vault.oracle().decimals(), kerosine vaults do not have an oracle and when a kerosine vaults tries to call an oracle function the transaction will fail.

Proof of Concept

1. User buys DNFT.
2. User adds Kerosine vault to the Vault Manager.
3. User approves Vault Manager to spend kerosene tokens on its behalf
4. User deposits 10 kerosine tokens to the kerosine vault
5. After a period of time, user tries to withdraw kerosine tokens from the kerosine vault.
6. User is unable to withdraw kerosine tokens from the kerosine vault.

Proof of Code

   // This is a forked test
  address user3 = address(0x8f10ae3ffD993bFb0A71E58E1bb606439dD5b301);

  function test_failedWithdrawForUnboundedKerosine() public {
        vm.deal(user3, 100 ether);
        uint256 amount = 10e18;

        Licenser licenser = Licenser(MAINNET_VAULT_MANAGER_LICENSER);
        vm.prank(MAINNET_OWNER);
        licenser.add(address(contracts.vaultManager));

        vm.prank(MAINNET_OWNER);
        contracts.kerosineManager.add(address(contracts.unboundedKerosineVault));

        vm.startPrank(user3);

        uint256 id = DNft(MAINNET_DNFT).mintNft{value: 1 ether}(address(user3));

        contracts.vaultManager.addKerosene(id, address(contracts.unboundedKerosineVault));

        ERC20(MAINNET_KEROSENE).approve(address(contracts.vaultManager), amount);
        contracts.vaultManager.deposit(id, address(contracts.unboundedKerosineVault), amount);

        vm.roll(block.timestamp + 100);

        vm.expectRevert();

        contracts.vaultManager.withdraw(id, address(contracts.unboundedKerosineVault), amount, user3);

        vm.stopPrank();
    }

Proof of Code Response

https://gist.github.com/Josh4324/2c754c6aa57f7307a4a0226b091a0f39

Tools Used

Manual Review

Recommended Mitigation Steps

Another withdraw function can be created for the unbounded kerosine vault or the current withdraw function can be adjusted to take into account the unbounded kerosine vault.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L269 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.sol#L60 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.unbounded.sol#L50

Vulnerability details

Impact

Users are unable to mint dyad stable coin when they add kerosine vault to their notes in addition to other non kerosine vaults. When users do not add kerosine vault to their notes, they are able to mint dyad stable-coins without any issues. This issue can be traced to the getKeroseneValue function.

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

 function getUsdValue(
    uint id
  )
    public
    view 
    returns (uint) {
 @>     return id2asset[id] * assetPrice() / 1e8;
  }

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

When the user tries to mint dyad tokens, the mintDyad function calls the getKeroseneValue at a point in order to calculate the totalUsdValue for all kerosene vault. The problem is that after the keroseneManager checks that the vault is licensed, the function vault.getUsdValue(id) was called in order to get the usdValue of the vault which then calls the assetPrice() function. This function makes another call to the vault oracle but kerosene vaults do not have an oracle function.

Only non kerosine vaults have an oracle function because it is needed to get the current price of the underlying assets. The transaction will revert when any kerosine vault tries to call the oracle function which does not exists in kerosine vaults contract.

Proof of Concept

1. User buys DNFT.
2. User adds Kerosine vault to the Vault Manager.
3. User approves Vault Manager to spend kerosene tokens on its behalf
4. User deposits 10 kerosine tokens to the kerosine vault
5. User adds Eth vault to the Vault Manager
6. User approves Vault Manager to spend weth tokens on its behalf
7. User deposits 1 weth token to the eth vault.
8. User tries to mint DYAD
9. User is unable to mint DYAD stablecoin

Proof of Code

// This is a forked test
address user3 = address(0x8f10ae3ffD993bFb0A71E58E1bb606439dD5b301);

function test_MintFailed() public {
        vm.deal(user3, 100 ether);
        uint256 amount = 10e18;
        uint256 mintAmount = 2000 ether;

        Licenser licenser = Licenser(MAINNET_VAULT_MANAGER_LICENSER);
        vm.prank(MAINNET_OWNER);
        licenser.add(address(contracts.vaultManager));

        vm.prank(MAINNET_OWNER);
        contracts.kerosineManager.add(address(contracts.unboundedKerosineVault));

        vm.startPrank(user3);

        uint256 id = DNft(MAINNET_DNFT).mintNft{value: 1 ether}(address(user3));

        contracts.vaultManager.addKerosene(id, address(contracts.unboundedKerosineVault));
        ERC20(MAINNET_KEROSENE).approve(address(contracts.vaultManager), amount);
        contracts.vaultManager.deposit(id, address(contracts.unboundedKerosineVault), amount);

        contracts.vaultManager.add(id, address(contracts.ethVault));
        ERC20(MAINNET_WETH).approve(address(contracts.vaultManager), amount);
        contracts.vaultManager.deposit(id, address(contracts.ethVault), 1 ether);

        vm.expectRevert();

        contracts.vaultManager.mintDyad(id, mintAmount, user3);

        vm.stopPrank();
    }

Proof of Code Response https://gist.github.com/Josh4324/67c843755a4520bc58c41ad6a61bcf89

Tools Used

Manual Review

Recommended Mitigation Steps

The assetPrice() function should be take the kerosine vaults into consideration in the calculation of the asset price.

Assessed type

DoS