DYAD - asui's results

Lines of code

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

Vulnerability details

Impact

The calls to withdraw function can be dos by anyone using deposit function with 0 token transfers, which leads to stuck assets for those who want to withdraw.

Proof of Concept

Since the VaultManagerV2 contract uses idToBlockOfLastDeposit[id] = block.number; in deposit and withdraw function to prevent depositing and withdrawing at the same time(block). And the assets(eg. weth, wstETH) are ERC20 tokens which doesn't revert on 0 token transfers, an attacker who monitors the mempool can revert all withdraw transaction by frontrunning and calling the deposit function in the same block with 0 as amount.

When the attacker sees the withdraw transaction he can frontrun it by calling deposit which will update the idToBlockOfLastDeposit to the current block.number and when the withdrawer's transaction gets executed the check if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock(); will revert his withdrawal because the idToBlockOfLastDeposit is the current block. The attacker can repeat this everytime the targeted user tries to withdraw and spend only gas fees and 0 assets for this attack while the user is impossible to withdraw his assets.

Tools Used

manual

Recommended Mitigation Steps

In the deposit function use isDNftOwner(id) modifier instead of isValidDNft(id) so that no one but the owner can deposit which will prevent the attack.

Or in both the functions change block.number to block.timestamp which will have a very rare/impossible case of two transactions to have the same timestamp rather than block.number which can be the same for two different transactions.

-      idToBlockOfLastDeposit[id] = block.number;
+      idToBlockOfLastDeposit[id] = block.timestamp;

Or set a minimum amount that must be deposited when deposit is called.

Assessed type

DoS