DYAD - gumgumzum's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L230-L239 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L241-L248 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L241-L248 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L269-L286 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L67-L78 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L80-L91 https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L62-L65 https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L93-L95

Vulnerability details

Impact

Users can increase their collateralization ratio allowing them to bypass the 150% collateralization ratio requirement and mint DYAD tokens corresponding to a 100% of their collateral value.

Proof of Concept

Root Cause

The current deployment scripts adds wSTETH and WETH vaults to both KerosineManager and VaultLicenser.

This allows users to add the same vault as both a non kerosene and a kerosene vault to their DNFT making the same deposited collateral count twice in the collatRatio calculation.

Scenarios

Bypass minimum 150% collateralization ratio

Assuming a WETH price of $3000 :

• Bob adds WETH vault as a non kerosene vault to his DNFT.
• Bob adds WETH vault as a kerosene vault to his DNFT.
• Bob deposits 1 WETH to the WETH vault.
• Bob can now mint 3000 DYAD tokens (when he should only be able to get 2000 DYAD tokens at most) because the collateralization check will pass since :
  • Collateralization Ratio = (Total Collateral USD Value / 3000) * 1e18
  • Collateralization Ratio = ((Total Non Kerosene USD Value + Total Kerosene USD Value) / 3000) * 1e18
  • Collateralization Ratio = ((3000 + 3000) / 3000) * 1e18
  • Collateralization Ratio = 2e18
  • 2e18 > 1.5e18

Tools Used

Manual Review

Recommended Mitigation Steps

Create a Kerosene Licenser and use it instead of using the Kerosene Manager for both managing and licensing kerosene vaults.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.unbounded.sol#L50-L67 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.bounded.sol#L44-L50 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.sol#L60-L67 https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L95 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L269-L286 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L230-L239 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L230-L239 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L156-L169 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L184-L202 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L205-L228

Vulnerability details

Impact

• Malicious users can escape liquidation.
• Potential DDoS of user either through regular use or malicious back running.

Proof of Concept

Root Cause

The asset price of Kerosine Vaults is based on their TVL and the current total supply of DYAD.

The TVL of the newly deployed vaults is 0 while the current total supply of DYAD is 622967.4.

This will lead to an underflow here.

Scenarios

Escaping liquidation

• Bob adds WETH and wSTETH vaults to his DNFT
• Bob deposits collateral to both vaults for his DNFT
• Bob mints DYAD based on the total collateral of his DNFT
• Bob adds UnboundedKerosineVault vault to his DNFT
• If Bob becomes liquidatable in the future, liquidation will fail because of the described underflow (VaultManagerV2@liquidate -> VaultManagerV2@collatRatio -> VaultManagerV2@getNonKeroseneValue -> UnboundedKerosineVault@getUsdValue -> UnboundedKerosineVault@assetPrice)

User DDoS

• Bob adds UnboundedKerosineVault vault to his DNFT
• Bob deposits collateral to the vault for his DNFT or Alice backruns and deposits a dust amount for Bob's DNFT
• Bob cannot mint or withdraw due to the described underflow (VaultManagerV2@mintDyad/withdraw -> VaultManagerV2@getNonKeroseneValue -> UnboundedKerosineVault@getUsdValue -> UnboundedKerosineVault@assetPrice).
• Also, Bob cannot remove the UnboundedKerosineVault for his DNFT since it has associated collateral in that vault, effectively rendering that DNFT unusable.

Tools Used

Manual Review

Recommended Mitigation Steps

• Delay licensing Kerosene Vaults in the new vault manager until the migration is complete
• Add the old vault TVL in the UnboundedKerosineVault asset price calculation

Assessed type

Under/Overflow

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/script/deploy/Deploy.V2.s.sol#L36-L113 https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.bounded.sol#L44-L50

Vulnerability details

Impact

Calls to BoundedKerosineVault@assetPrice will fail forcing the multi-sig wallet to update the unboundedKerosineVault for the deployed BoundedKerosineVault afterwards.

This contradicts the Migration section in the contest Readme :

The whole migration is described in Deploy.V2.s.sol. The only transaction that needs to be done by the multi-sig after the deployment is licensing the new Vault Manager.

Proof of Concept

  function assetPrice() 
    public 
    view 
    override
    returns (uint) {
      return unboundedKerosineVault.assetPrice() * 2; // <--- @audit
  }

The call to assetPrice will fail because the unboundedKerosineVault is not set (.i.e is still address(0)).

Tools Used

Manual Review

Recommended Mitigation Steps

Set unboundedKerosineVault for the deployed BoundedKerosineVault in the deployment script.

Assessed type

Other