DYAD - Tychai0s's results

Lines of code

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

Vulnerability details

Impact

<description>An attacker can block a user from withdrawing by front-running the withdraw transaction and depositing 0 amount to the NFT.</description>

Proof of Concept

In VaultManagerV2.sol:127, the deposit function updates the idToBlockOfLastDeposit mapping with the current block number for the given NFT id. The withdraw function at line 143 checks if the idToBlockOfLastDeposit for the given id is equal to the current block number, and if so, it reverts the transaction with the "DepositedInSameBlock" error.

An attacker can exploit this by front-running the user's withdrawal transaction and calling the deposit function with a 0 amount for the same NFT id. Since there is no check for a minimum deposit this will update the idToBlockOfLastDeposit mapping to the current block number, causing the user's subsequent withdrawal transaction to revert due to the "DepositedInSameBlock" error.

Tools Used

Manual Review

Recommended Mitigation Steps

A simple solution for this would be to have a minimum deposit amount on VaultManagerV2.deposit function as it would be not profitable for an attacker to lose funds just to revert the user transaction and make them wait for one block.

Issue Type

MEV

Assessed type

MEV

Lines of code

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

Vulnerability details

Impact

In the Kerosene Manager only non-kerosene vaults are allowed, otherwise a recursion is created when calculating the price of Kerosene but then VaultManagerV2 requires the Kerosene vault to be added to the manager before being able to add it as a vault for an NFT.</description>

Proof of Concept

In VaultManagerV2.sol:74, the addKerosene function allows adding Kerosene vaults to the vaultsKerosene array. However, the assetPrice function in Vault.kerosine.unbounded.sol is meant to get only non-Kerosene vaults from the kerosineManager.getVaults() function. If Kerosene vaults are added to the KeroseneManager, it will create an infinite recursion when vault.assetPrice() is called within the loop, as the Kerosene vault's assetPrice function will try to calculate the Kerosene price again, leading to a stack overflow.

// VaultManagerV2.sol
function addKerosene(uint256 id, address vault) external isDNftOwner(id) {
    if (vaultsKerosene[id].length() >= MAX_VAULTS_KEROSENE) revert TooManyVaults();
	if (!keroseneManager.isLicensed(vault)) revert VaultNotLicensed(); // Needs Kerosene Manager to have Kerosene Vault added
	if (!vaultsKerosene[id].add(vault)) revert VaultAlreadyAdded();
    emit Added(id, vault);
}

// Vault.kerosine.unbounded.sol
function assetPrice() public view override returns (uint256) {
	uint256 tvl;
	address[] memory vaults = kerosineManager.getVaults();
	uint256 numberOfVaults = vaults.length;
	// @audit-info this is meant to get only Non-kerosene vaults, otherwise it will eventually revert when calling the vault.assetPrice() function because of infinite recursion.
	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());
	}
    // @audit-info C = tvl, D = dyad.totalSupply, K = kerosineDenominator.denominator
	// @audit numerator can be negative (revert) if tvl price falls below dyad.totalSupply
	// @audit-info given that in order to mint dyad a 150% collateral is required, even if nobody uses
	uint256 numerator = tvl - dyad.totalSupply();
	uint256 denominator = kerosineDenominator.denominator();
    // @audit-info this assetPrice goes higher if the TVL increases in vaults or if dyad.totalSupply decreases
    return numerator * 1e8 / denominator;
}

Tools Used

Manual Review

Recommended Mitigation Steps

The simplest way would be to use a VaultLicenser in VaultManagerV2 instead of KeroseneManager, but special attention must be put in making sure they are in sync regarding the vaults added.

Assessed type

Other