DYAD - Sabit's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L215-L215

Vulnerability details

Impact

It is impossible to liquidate a user with the highest minted DYAD when such a user is undercollateralized

Proof of Concept

To liquidate an undercollateralized user, the liquidator must substract the equivalence of the minted DYAD of the user being liquidated from his balance.

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

And here is the burn function:

  function burn(
      uint    id, 
      address from,
      uint    amount
  ) external 
      licensedVaultManager 
    {
      _burn(from, amount);
      mintedDyad[msg.sender][id] -= amount;
  }

What the above means is that the total minted DYAD burnt from the user being liquidated (dyad.mintedDyad(address(this), id)) is the same amount that must be substracted from the liquidator.

The liquidate function does not provide for liquidation in small amounts.

The problem this design creates is that when a user with a high or the highest minted DYAD amount is undercollateralized, it's impossible for anyone to liquidate the user. This is because no one would have the needed DYAD tokens to liquidate the undercollateralized user.

Tools Used

Manual review

Recommended Mitigation Steps

I suggest that liquidating in small amount should be implemented.

Assessed type

Context

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L119-L154 https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L42-L44

Vulnerability details

Impact

Arbitrary user can temporarily block owner from withdrawing

Proof of Concept

The withdraw function prevents withdrawals in the same block as a deposit. This allows anyone to DoS the owner by making ridiculously small deposits to block withdrawals.

Note that anyone can call the deposit function since the isValidDNft modifier only checks for a valid "id" and not the owner.

Tools Used

Manual review

Recommended Mitigation Steps

I suggest a check that if deposit is not done by the owner, it shouldn't block the owner from calling the withdraw function.

Assessed type

DoS

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L205-L229

Vulnerability details

Impact

The protocol can be undercollateralized and users not being able to withdraw their funds.

Proof of Concept

Liquidators call the liquidate functio for the profit and not for anything else. There is 20% bonus for every liquidation. However, there are cases of small positions where a user only has $2, $3 worth of value in their position.

Liquidators won't want to liquidate such positions because the position value doesn't even cover gas fees.

These low value acccounts will never get liquidated leaving the protocol with bad debt.

Tools Used

Manual review

Recommended Mitigation Steps

I suggest allowing users to mint if their collateral value is above a particular threshold.

Assessed type

Error