An attacker can block withdraw method permanently for all ids for each block

[c4-bot-4]

Lines of code

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

Vulnerability details

Impact

withdraw method can be blocked permanently for all ids for each block.

Proof of Concept

depsoit amount for a note id by design can't be withdrawn immediately, The user should wait for the next block to be able to withdraw his amount, Ethereum generate new block every 12 seconds, meaning the user who make a deposit he should wait at least a 12 seconds to be able to withdraw, which is on purpose as the protocol made this protection against flash loan attacks or any deposit and withdraw in one time transaction.

However, because of this condition idToBlockOfLastDeposit[id] = block.number, it can cause DoS and block the withdraw method, a simple scenario that Bob can call deposit method and pass any note id with 1 WEI of depsoit amount to any Vault:

A simple scenario:

• Bob pass Alex's note id which is 5.
• Bob deposit only 1 WEI
• Deposit succeed and block.number now stored in idToBlockOfLastDeposit at id 5.
• Alex call withdraw but revert is done idToBlockOfLastDeposit[id] == block.number
• Alex is blocked now and need to wait for the next block.

It seems a short time to wait, and Alex wasn't blocked for long time. However, this unfortunately could not be the case, Bob could use this bug to achieve a full block for many ids for many consecutive blocks.

The critical scenario:

• Bob will create a vault contract (BVault).
• Bob will create a contract with a method to call deposit with 1 WEI amount and pass his vault address within a loop of all note ids.

  for (uint i = 0; i < Ids; i++) {
      VaultManagerV2.deposit(Ids[i], BVault, 1 WEI);
  }

• Calls will be done to his vault since there is no check for licensed vault on deposit method.
• Withdrawals for all ids will be blocked on withdraw until the next block.
• Bob will repeat above for each next block.
• The protocol withdrawals will be blocked for next blocks until Bob stop.

This attack will cost nothing except the TX fee.

Tools Used

Manual Review

Recommended Mitigation Steps

• Remove the condition and use different implementation or keep the condition but allow the deposit only for valid nft owner.

Assessed type

DoS

[c4-pre-sort]

JustDravee marked the issue as duplicate of #1103

[c4-pre-sort]

JustDravee marked the issue as duplicate of #489

[c4-pre-sort]

JustDravee marked the issue as sufficient quality report

[c4-judge]

koolexcrypto marked the issue as not a duplicate

[c4-judge]

koolexcrypto marked the issue as duplicate of #1266

[c4-judge]

koolexcrypto marked the issue as satisfactory

[c4-judge]

koolexcrypto marked the issue as duplicate of #930