DYAD - okolicodes's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/VaultManager.sol#L63-L73

Vulnerability details

Impact

In the remove function vaults can only be removed when they are no token assets remaining in the vault. This can lead to a lot of issues as an attacker can keep on sending dust amounts of token assets to all the vaults in the protocol making it impossible to close vaults in different dNft positions

Proof of Concept

Supposing a user has created the max number of vaults allowed (5) and intends on removing a vault with asset A and adding a new vault with asset B. An attacker can deposit dust amounts of asset A to make it impossible for the vault owner to remove the vault and make changes as they deem fit and the attacker could do this over and over again. They can also perform this attack on a larger scale as they can send dust amounts of vault assets to a whole lot of vaults in the protocol forever preventing users from removing vaults and this is unintended behaviour that leads to bad user experience with the protocol and DoS.

  {
  >>>  if (Vault(vault).id2asset(id) > 0) revert VaultHasAssets();
    if (!vaults[id].remove(vault))     revert VaultNotAdded();
    emit Removed(id, vault);
  }

Tools Used

Detailed Attention

Recommended Mitigation Steps

Close vaults irrespective of any tokens inside and send any remaining tokens to the vault owner.

Assessed type

DoS