DYAD - windhustler's results

Lines of code

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

Vulnerability details

Impact

The current deployment setup enables permanent DoS attack on the VaultManagerV2 contract. The assetPrice() function in the UnboundedKerosineVault.sol contract will keep reverting until the tvl surpasses the total supply of DYAD tokens. This makes the VaultManagerV2 contract unusable.

Proof of Concept

By observing the assetPrice() function in the Vault.kerosine.unbounded.sol file, we can see that the numerator is the difference between tvl and dyad total supply. tvl is calculated as the sum of all the assets in the vaults that were whitleisted in the KerosineManager contract.

## UnboundedKerosineVault.sol

    function assetPrice() public view override returns (uint256) {
        uint256 tvl;
>>>        address[] memory vaults = kerosineManager.getVaults();
        uint256 numberOfVaults = vaults.length;
        for (uint256 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());
        }
>>>        uint256 numerator = tvl - dyad.totalSupply();
        uint256 denominator = kerosineDenominator.denominator();
        return numerator * 1e8 / denominator;
    }

On the other had the deployment script DeployV2 deploys new weth and wsteth vaults to be used with the new VaultManagerV2 contract. It uses already existing DYAD contract on mainnet. At the time of writing this report, the totalSupply of DYAD is: 632967400000000000000000 or 632967.4 DYAD tokens.

As both vaults are deployed with 0 balance, the assetPrice() function will revert because the tvl is less than the dyad.totalSupply(). It will keep reverting until the tvl is greater than the dyad.totalSupply().

As assetPrice() from UnboundedKerosineVault.sol is used in the VaultManagerV2 contract, as part of getKeroseneValue() function, almost all the functions in the VaultManagerV2 contract will revert. This makes mintDyad function unusable.

There is no clear migration path from the old VaultManager to the new VaultManagerV2 contract with the current setup.

The existing vaults, i.e. WETH_VAULT and WSTETH_VAULT have the following logic:

contract Vault is IVault {
    using SafeTransferLib for ERC20;
    using SafeCast for int256;
    using FixedPointMathLib for uint256;

    uint256 public constant STALE_DATA_TIMEOUT = 90 minutes;

    IVaultManager public immutable vaultManager;
    ERC20 public immutable asset;
    IAggregatorV3 public immutable oracle;

    mapping(uint256 => uint256) public id2asset;

    ...

    function withdraw(uint256 id, address to, uint256 amount) external onlyVaultManager {
>>>        id2asset[id] -= amount;
        asset.safeTransfer(to, amount);
        emit Withdraw(id, to, amount);
    }

    ... 
}

contract VaultManager is IVaultManager {

    ...
    
    function withdraw(uint256 id, address vault, uint256 amount, address to) public isDNftOwner(id) {
>>>        Vault(vault).withdraw(id, to, amount);
        if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();
    }

    ...
}

• vaultManager is immutable in both vaults. Withdrawal can only be done by the currently deployed VaultManager contract.
• There is no way of forcing the withdrawal of the assets from the vaults.

Also, the VaultManager can't simply be unlicensed from the DYAD contract because then users can't burn their DYAD tokens and withdraw the assets from the vaults.

To get back to the initial claim, until the tvl in the new vaults surpasses the total supply of DYAD, the new VaultManagerV2 contract will keep reverting.

This is a permanent DoS attack on the VaultManagerV2 contract as the old VaultManager can't be unlicensed from the DYAD contract and malicious griever can keep on minting DYAD through the old VaultManager contract. As a consequence, the new VaultManagerV2 contract will keep reverting.

Tools Used

• Manual review

Recommended Mitigation Steps

Devise a migration plan from the old VaultManager to the new VaultManagerV2 contract that takes into account the existing vaults and the DYAD token holders.

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

Kerosene price depends on total value locked inside the regular vaults which makes it subject to manipulation. This can be used in a bat and switch attack to liquidate users. It can also be used to mint DYAD tokens with much lower actual collateralization ratio than 150%. In turn, this can leave the protocol in undercollateralized state.

Proof of Concept

The new VaultManagerV2 contract has introduced the Kerosene vaults. Users has the following possibility:

• The amount of DYAD they can mint has to be smaller than the amount of collateral deposited in the non-Kerosene vaults.
• Kerosene deposited inside the Kerosene vaults act as additional collateral not to get liquidated.

    /// @inheritdoc IVaultManager
    function mintDyad(uint256 id, uint256 amount, address to) external isDNftOwner(id) {
        uint256 newDyadMinted = dyad.mintedDyad(address(this), id) + amount;
>>>        if (getNonKeroseneValue(id) < newDyadMinted) revert NotEnoughExoCollat(); // So minted dyad is capped by the value of your collateral. How is that over collateralized?
        dyad.mint(id, to, amount);
>>>        if (collatRatio(id) < MIN_COLLATERIZATION_RATIO) revert CrTooLow();
        emit MintDyad(id, amount, to);
    }

An example of how this work is as follows:

• User deposits $1000 worth of ETH collateral in the ETH vault.
• The price of Kerosene is $1, and he deposits 500 Kerosene tokens in the Unbounded Kerosene vault.
• Based on this he can mint 999.999 DYAD tokens.
• His collateral ratio is slightly above 150%.

If the price of ETH drops or the price of Kerosene drops, he will be subject to liquidation.

Price of ETH can't be manipulated, so it is reasonable to he gets liquidated if it drops, and he is expected to monitor his position and provide additional collateral if needed.

However, the price of Kerosene is subject to manipulation. A malicious actor can set up a bait and switch attack to liquidate the users.

1. Kerosene price depends on the difference between the total value locked inside the vaults and the total supply of DYAD tokens.
2. The attacker can deposit large amounts of assets inside the non-Kerosene vaults to artificially inflate the Kerosene price.
3. It's important to mention that he deposits under another NFT ID, so he can easily withdraw the assets later.
4. This will increase the collateralization ratio of the users that use the Kerosene as part of their collateral.
5. Now the attacker needs to wait for the price of collateral in non-Kerosene vaults to drop.
6. As soon as the price drops, the attacker can remove the assets from the non-Kerosene vaults.
7. Plummeting the price of Kerosene and liquidating the users.

Another issue with this manipulation property of Kerosene is that users are able to mint DYAD tokens with 1:1 colateralization ratio for ETH and WSTETH. While the additional 50% can be provided by Kerosene. Given the inflated price of Kerosene detailed in the description above, attacker can mint DYAD with much lower actual collateralization ratio than 150%. This is a serious issue because it means that if the price of actual assets(ETH and WSTETH) drops, it leaves the protocol in undercollateralized state.

Tools Used

• Manual review

Recommended Mitigation Steps

One of the possibilities for tackling the issue is requiring non-Kerosene collateral to be above 100%. It can have a value of 120% for example while the rest can be provided with Kerosene.

Assessed type

Other