DYAD - LeoGold's results

Lines of code

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

Vulnerability details

Impact

VaultManagerV2::collatRatio() used the totalUsdValue of the collateral of the specified Id to determine the calculation, user who made deposit and have addedv the same vault in both VaultManagerV2::add() and VaultManagerV2::addKerosene() will have their collateral being referenced twice in collatRatio calculation. (this is possible because weth and westh are added in both kerosenemanager.sol and licenser.sol)

VaultManagerV2::getNonKeroseneValue() and VaultManagerV2::getKeroseneValue() both individually referenced the same collateral and return the same value seperately which was then added together in VaultManagerV2::getTotalUsdValue(). this will raise the collateral ratio calculation to keep returning a value above 150% of their collateral and can never get liquidated, which in the real case the collateral to the dyadMinted ratio is 100% or below. This will break the protocol invariant of TVL > dyad

• TVL == dyad at the point of minting 100% collateral,
• TVL < dyad when collateral falls in price because it cannot be liquidated when cr keeps returning double of the actual ratio

Proof of Concept

TotalUsd Calculation

collateral ratio

kerosene manager adding the two collateral here in the deploy script

Vault Licenser adding the two collateral here also in the deploy script

github gist for POC here

copy and paste file in test/fork folder and run with forge t --mt testCollateral_Ratio_Return_Double -vv

Tools Used

foundry

Recommended Mitigation Steps

The team should mitigate this risk by making sure the calculation of TotalUsdValue is not returning double of the supposed value

Assessed type

Context