DYAD - d3e4's results

Lines of code

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

Vulnerability details

Impact

Only the exact entire DYAD minted amount can be liquidated, which means that there would be few holder's able to liquidate large positions, and no one can liquidate the largest position.

Proof of Concept

VaultManagerV2.liquidate() burns the entire amount of minted DYAD by id:

dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));

This is a severe limitation that requires the liquidator to have as much DYAD as the liquidatee has minted. Especially, a whale can ensure he has minted more DYAD than any one else owns, and he can therefore never be liquidated.

Recommended Mitigation Steps

Allow partial liquidations.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

VaultManagerV2.redeemDyad() reverts for KerosineVaults, and uses incorrect decimals.

Proof of Concept

VaultManagerV2 now includes KerosineVaults in addition to the regular Vaults. Vault has an oracle which returns the assetPrice(), but KerosineVault does not have an oracle and uses a different price calculation. The decimals of Vault.assetPrice() is oracle.decimals(), but the decimals of UnboundedKerosineVault.assetPrice() (and BoundedKerosineVault.assetPrice()) is 1e8.

VaultManagerV2.redeemDyad() calls _vault.oracle() for any _vault. It is possible to deposit into KerosineVaults. VaultManagerV2.redeemDyad() will therefore revert on calling _vault.oracle() on a KerosineVault, and more generally uses the wrong decimal calculation.

Recommended Mitigation Steps

In VaultManagerV2.redeemDyad() replace _vault.oracle().decimals() by a variable assetPriceDecimals which if _vault is a Vault is _vault.oracle().decimals(), but if _vault is a UnboundedKerosineVault or BoundedKerosineVault is 1e8. Consider adding a function in Vault.Kerosine.sol which returns the assetPriceDecimals such that this is consistent at the parent level.

Or, add a dummy oracle, with a decimals() function (in this case returning 1e8), to KerosineVault such that this can be invoked as it already is in redeemDyad().

Assessed type

DoS

Lines of code

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

Vulnerability details

Impact

The liquidator does not get the 20% bonus on the liquidated amount as promised. Rather he receives 20% of the excess collateral. This means that the unhealthier the debt the less incentive there is to liquidate. This may lead to less incentivized liquidatable positions to be neglected until they are entirely unprofitable to liquidate. Considering the gas cost to liquidate the collateral position to liquidate must also be at least twice as large to be profitable, compared to the promised 20% bonus. This facilitates the opening of positions which can never be profitably liquidated. Note also that undercollateralized positions still burns the entire minted amount of DYAD, so that it cannot ever be profitably liquidated.

The reason for the High rating is that positions can be opened which can only be liquidated at a loss for the liquidator.

Proof of Concept

The liquidator is promised "an equivalent value plus a 20% bonus of the target Note’s backing colateral". The calculation in VaultManager.liquidate) is

uint cr = collatRatio(id);
if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();
dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));

uint cappedCr               = cr < 1e18 ? 1e18 : cr;
uint liquidationEquityShare = (cappedCr - 1e18).mulWadDown(LIQUIDATION_REWARD);
uint liquidationAssetShare  = (liquidationEquityShare + 1e18).divWadDown(cappedCr);

uint numberOfVaults = vaults[id].length();
for (uint i = 0; i < numberOfVaults; i++) {
    Vault vault      = Vault(vaults[id].at(i));
    uint  collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);
    vault.move(id, to, collateral);
}

where LIQUIDATION_REWARD is 20%. That is, the liquidator receives collateral * ((collateral/dyad - 1) * 0.2 + 1) / (collateral/dyad), where we count the value rather than nominal amounts and collateral is the total collateral of the liquidatee and dyad is the DYAD minted by the liquidatee. This simplifies to dyad + 0.2 * (collateral - dyad). That is, the liquidator receives only 20% of the excess collateral.

Since the collateral ratio must be less than 1.5 in order to be liquidatable the bonus is less than 10% of the liquidated amount, so the amount of DYAD liquidated must be more than ten times greater than the gas cost of a liquidation. The gas cost of a liquidation would probably be on the order of $10 which means that one could mint $100 DYAD without risk of being liquidated. These positions can be set at peak values of the collateral. This gives a significant advantage in terms of collateral requirement.

Since the entire minted amount of DYAD is burnt from the liquidator undercollateralized positions cannot be profitably liquidated even without considering the gas cost.

Recommended Mitigation Steps

Calculate the liquidationAssetShare as 1.2 times the value of the burnt DYAD. Also consider reducing the amount of DYAD burnt from the liquidator in case the collateral ratio is less than 1.2, so that there is always a 20% incentive to liquidate. This can be solved as follows.

uint liquidationAssetShare  = (LIQUIDATION_REWARD + 1e18).divWadDown(cr);
uint shareToBurn = 1e18;
if (liquidationAssetShare > 1e18) {
    shareToBurn = (1e18).divWadDown(liquidationAssetShare);
    liquidationAssetShare = 1e18;
}
dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id).mulWadDown(shareToBurn));

In terms of value this calculation gives collateral * 1.2 * dyad/collateral = 1.2 * dyad of collateral to the liquidator. If this is more than available in collateral the liquidator only pays dyad / (1.2 * dyad/collateral) = collateral / 1.2 in DYAD for the full collateral.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/script/deploy/Deploy.V2.s.sol#L93-L96 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L88 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManagerV2.sol#L280 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/KerosineManager.sol#L50

Vulnerability details

Impact

As much DYAD can be minted as collateral deposited. The KerosineVaults cannot be withdrawn from.

Proof of Concept

In VaultManagerV2.sol add() is used to add a non-KerosineVault, and addKerosene() is used to add a KerosineVault. These need to be kept separate so that getNonKeroseneValue() and getKeroseneValue() can be calculated. The user can add() a non-KerosineVault only if it isLicensed() in the vaultLicenser, and addKerosene() a KerosineVault only if it isLicensed() in the keroseneManager. However, in the deploy script the non-KerosineVaults ethVault and wstEth are added to the kerosineManager and all vaults are added to the VaultManagerV2's vaultLicenser.

This means that a user can add() any vault, and addKerosene() only ethVault and wstEth.

Open critically collateralized position

The user can add() e.g. ethVault, and also addKerosene() ethVault. Then getTotalUsdValue() will double count the amount deposited into this vault. This enables the user to mint as much DYAD as collateral deposited. More is prevented by the check that getNonKeroseneValue(id) >= newDyadMinted.

Brick the KerosineVaults

KerosineVault.assetPrice() loops over the vaults in kerosineManager, i.e. ethVault and wstEth, in its sum to tvl. Since ethVault is critically collateralized any drop in price will cause there to be less tvl than minted DYAD, and the line uint numerator = tvl - dyad.totalSupply(); reverts.

assetPrice() is called on withdrawals, and indirectly also on minting DYAD if they have added a KerosineVault. This means users can deposit into the KerosineVaults but not withdraw or mint DYAD.

Recommended Mitigation Steps

By design the KerosineManager should know only the non-KerosineVaults for a correct valuation of Kerosene. So the adding of ethVault and wstEth to kerosineManager in the deploy script is correct. However, VaultManagerV2.addKerosene() checks keroseneManager.isLicensed which checks for these same vaults. This is incorrect. This then needs to be fixed either by having another list of the KerosineVaults in the KerosineManager which is the one checked by KerosineManager.isLicensed(), or by using another Licenser, known by VaultManagerV2, which stores the licensed KerosineVaults. In the latter case the check in getKeroseneValue() has to be amended accordingly. Change Deploy.V2.s.sol accordingly.

vaultLicenser should only license non-KerosineVaults. So vaultLicenser.add(address(unboundedKerosineVault)); should be removed from Deploy.V2.s.sol.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

The valuation of Kerosene can be manipulated such that it is possible to increase the collateral ratio of other's positions, such that they may mint more DYAD, and then suddenly decrease the collateral ratio to cause their positions to become liquidatable. These can then be liquidated for profit.

Proof of Concept

The collateral ratio is the ratio of total collateral value, summed over all Vaults and KerosineVaults, to amount of minted DYAD. This value is determined by the vault's assetPrice(). In the case of the KerosineVaults this is proportional to tvl - dyad.totalSupply(). tvl is the total value locked in all other non-KeroseneVaults. An attacker can therefore increase this assetPrice() by depositing collateral in e.g. the wETH Vault, without minting any DYAD. Since the price is increased the collateral ratios of all users who have deposited into the KerosineVault will increase. They may thus be inclined to mint more DYAD, still keeping a safe margin against liquidatability. Later when many users have done so, the attacker can mint as much DYAD as his collateral allows, which decreases the assetPrice() which in turn decreases the collateral ratio of the users. Done with a sufficient amount of funds, many users a likely to now be liquidatable. The attacker can then immediately liquidate them for profit.

As an example suppose tvl is $1 million and total supply of DYAD is 0.5 million (approximately the actual current figures). Let's say that this assetPrice() is then 0.5. Suppose some, the most vulnerable, users have half of their collateral value in KeroseneVaults, and that they also keep a collateral ratio around 2. The attacker deposits $1 million in collateral. This triples the assetPrice() of the KeroseneVaults to 1.5 and said users' collateral ratios double to 4, and they start minting more DYAD until their collateral ratios are back down to 2. The attacker can then suddenly mint DYAD himself, up to 0.667 million, which then reduces the assetPrice() to 0.833. This immediately reduces these users' collateral ratios to 1.33, and they are now liquidatable. Note that when the users mint DYAD this also decreases the assetPrice(), but it can be assumed to affect it much less than the attacker's minting, who also can ensure an arbitrarily large margin as well as time his minting to target the most vulnerable.

Recommended Mitigation Steps

The most an attacker can reduce the price by minting DYAD is down to a third. Therefore consider using a different collateral ratio calculation to determine how much DYAD a user is allowed to have in proportion to his collateral, i.e. when minting or withdrawing. Instead of (NonKeroseneValue + KeroseneValue) / mintedDyad use (NonKeroseneValue + KeroseneValue/3) / mintedDyad This effectively requires a collateral ratio of 4.5 on the KeroseneValue part, such that if the price is reduced to a third it is still not liquidatable. The collateral ratio to determine whether liquidatable should remain the same.

Assessed type

Other