DYAD - lionking927's results

Lines of code

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

Vulnerability details

Impact

deposit function in VaultManagerV2 contract doesn't check zero amount so it can cause to fail normal withdraw operation if attacker find normal withdraw tx in pending pool and make zero amount deposit tx.

Proof of Concept

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L127 In the above line, deposit function of VaultManagerV2 updates idToBlockOfLastDeposit[id] as block number even though the given amount is zero. https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L143 so it can cause above checking code fails although it's normal withdraw operation if attacker create empty deposit when it founds any withdrawal operation in pending pool. of course this can take some gas for attackers but it can make the tx fails each time user wants to withdraw so it can reduce the trust of your service and while withdraw tx fails several times, attacker can do another kind of attacking as they know withdraw tx will be created again in next time, etc.

Tools Used

vscode, manual review

Recommended Mitigation Steps

Add zero amount value in deposit function or use nonReentrant modifier of ReentrancyGuard from openzeppelin-contracts for withdraw function.

Assessed type

DoS