DYAD - MiniGlome's results

Lines of code

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

Vulnerability details

Summary

The VaultManagerV2 prevents users from depositing and withdrawing funds in the same block with the same NFT id. The problem lies in the fact that it is not necessary to be the owner of the NFT to call the deposit() function. So, if you remove your assets from a vault a malicious actor can call the deposit() function in the same block as you called the withdraw() function. The malicious actor can therefore prevent anyone from withdrawing assets by front-running every withdraw() call.

Vulnerability Details

The idToBlockOfLastDeposit mapping is updated in the deposit() function and withdraws occurring in the same block (same block.number) will revert. The attacker can frontrun every withdraw() call with a deposit() call with the same NFT id. Furthermore, the attacker does not need to actually deposit funds during the deposit() call because he can use his own contract implementing the Vault interface or use a 0 amount.

Here is the affected code:

File: VaultManagerV2.sol

L119:   function deposit(
    uint    id,
    address vault,
    uint    amount
  ) 
    external 
      isValidDNft(id) // @audit Anyone can deposit with a valid NFT id
  {
    idToBlockOfLastDeposit[id] = block.number;
    Vault _vault = Vault(vault); // @audit The vault can be any arbitrary address
    _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
    _vault.deposit(id, amount);
  }

  /// @inheritdoc IVaultManager
L134:  function withdraw(
    uint    id,
    address vault,
    uint    amount,
    address to
  ) 
    public
      isDNftOwner(id)
  {
    if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();
    uint dyadMinted = dyad.mintedDyad(address(this), id);
    Vault _vault = Vault(vault);
    uint value = amount * _vault.assetPrice() 
                  * 1e18 
                  / 10**_vault.oracle().decimals() 
                  / 10**_vault.asset().decimals();
    if (getNonKeroseneValue(id) - value < dyadMinted) revert NotEnoughExoCollat();
    _vault.withdraw(id, to, amount);
    if (collatRatio(id) < MIN_COLLATERIZATION_RATIO)  revert CrTooLow(); 
  }

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

Impact

The withdraw mechanism can be subject to DOS by a malicious actor, preventing any user from withdrawing assets from the protocol's vaults.

Tools Used

Manual review

Recommendations

Either remove the interdiction to withdraw in the same bock, or only allow NFT owner to deposit in the Vault registered with this NFT (i.e. use the isDNftOwner(id) modifier for the deposit() function)

Assessed type

DoS