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

8 stars 6 forks source link

Malicious actor can prevent users from withdrawing #181

Closed c4-bot-10 closed 6 months ago

c4-bot-10 commented 6 months ago

Lines of code

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

Vulnerability details

Whenever a deposit is made in the VaultManagerV2 the block.number is registered. Whenever a withdrawal is being processed the block.number is being checked and cannot be the same as the the one registered in the deposit(), which means that deposit and withdrawal cannot happen in the same block for a user. The issue at hand is that a user can be denied from withdrawing if a malicious actor frontruns the user's withdrawals and deposits 1 wei of assets in the user's vault.

Impact

Users will experience withdraw denial.

Proof of Concept

The deposit() function will register when deposits are made into a vault.

  function deposit(
@>   uint    id,
     address vault,
     uint    amount
  ) 
    external 
      isValidDNft(id)
  {
@>  idToBlockOfLastDeposit[id] = block.number;

And withdraw()als revert when idToBlockOfLastDeposit[id] == block.number

  function withdraw(
    uint    id,
    address vault,
    uint    amount,
    address to
  ) 
    public
      isDNftOwner(id)
  {
  @> if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();

This means that anyone can deposit one wei worth of assets in order to constantly front run the withdrawer whenever he fires a withdrawal transaction.

Attack scenario

Bob wants to attack Eve, he sets up a bot that frontruns any of Eve's withdrawal requests.

Eve withdraws. Bob's bot deposits. Eve transaction reverts. Eve is sad

Tools Used

Recommended Mitigation Steps

In order to mitigate this issue consider adding a minimum deposit, this will make such attacks very costly, undoable.

  function deposit(
    uint    id,
    address vault,
    uint    amount
  ) 
    external 
      isValidDNft(id)
  {
+ require(amount > minDepositAmount, "deposit too small");

Assessed type

DoS

c4-pre-sort commented 6 months ago

JustDravee marked the issue as duplicate of #489

c4-pre-sort commented 6 months ago

JustDravee marked the issue as sufficient quality report

c4-judge commented 6 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

c4-judge commented 6 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

c4-judge commented 6 months ago

koolexcrypto marked the issue as nullified

c4-judge commented 6 months ago

koolexcrypto marked the issue as not nullified

c4-judge commented 6 months ago

koolexcrypto marked the issue as duplicate of #1001

c4-judge commented 6 months ago

koolexcrypto marked the issue as satisfactory

c4-judge commented 6 months ago

koolexcrypto changed the severity to 3 (High Risk)