code-423n4 / 2024-04-dyad-findings

8 stars 6 forks source link

Users can be prevented from withdrawing funds #606

Closed c4-bot-10 closed 4 months ago

c4-bot-10 commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The deposit() function is publicly accessible, allowing any user to deposit funds on behalf of another. However, the protocol includes an additional layer of security to prevent customers from depositing and withdrawing in the same block, tracked in the idToBlockOfLastDeposit mapping. An attacker can exploit this by monitoring a victim's activity and depositing a small amount on their behalf, thereby blocking their withdrawals. This could be financially motivated if the user is close to being liquidated and wants to close his positions.

Proof of Concept

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

   function deposit(
    uint    id,
    address vault,
    uint    amount
  ) 
    external 
      isValidDNft(id)
  {
    idToBlockOfLastDeposit[id] = block.number; 
    Vault _vault = Vault(vault);
    _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
    _vault.deposit(id, amount);
  }

Tools Used

Manual Analysis

Recommended Mitigation Steps

Multiple fixes could be implemented:

proposal 1: Restrict the protocol so users can deposit only for themselves. proposal 2: Make the mapping track the deposits of msg.sender and not the id.

Assessed type

DoS

c4-pre-sort commented 4 months ago

JustDravee marked the issue as duplicate of #489

c4-pre-sort commented 4 months ago

JustDravee marked the issue as sufficient quality report

c4-judge commented 4 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

c4-judge commented 4 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

c4-judge commented 4 months ago

koolexcrypto marked the issue as nullified

c4-judge commented 4 months ago

koolexcrypto marked the issue as not nullified

c4-judge commented 4 months ago

koolexcrypto marked the issue as duplicate of #1001

c4-judge commented 4 months ago

koolexcrypto marked the issue as satisfactory