Attacker can block all deposits to a vault with abusing the cooldown period

[hats-bug-reporter[bot]]

Github username: @0xfuje Twitter username: 0xfuje Submission hash (on-chain): 0x683539dba401843200192eb8ca7bd4a584fd75aa33808bc441ff41021b7102d5 Severity: high

Description:

Impact

Permanent denial of service of Vaults since no deposit will execute

Description

The _depositAndMint() function handles deposits for a given Vault, note that any user (or if whitelist is enabled any whitelisted user) can deposit for anyone by inputting their address in _depositFor param.

There is a cooldown for each users that deposits, see: _mintTokenAndSetCoolDown() inside _depositAndMint(). If the depositor's balance is zero, we will apply full cooldown in _calculateCoolDownPeriod().

PortfolioToken.sol - _mintTokenAndSetCoolDown()

  function _mintTokenAndSetCooldown(
    address _to,
    uint256 _mintAmount
  ) internal returns (uint256) {
    uint256 entryFee = assetManagementConfig().entryFee();

    if (_mintAndBurnCheck(entryFee, _to)) {
      _mintAmount = feeModule()._chargeEntryOrExitFee(_mintAmount, entryFee);
    }

    _mint(_to, _mintAmount);

    // Updates the cooldown period based on the minting action.
    userCooldownPeriod[_to] = _calculateCooldownPeriod(
      balanceOf(_to),
      _mintAmount,
      protocolConfig().cooldownPeriod(),
      userCooldownPeriod[_to],
      userLastDepositTime[_to]
    );
    userLastDepositTime[_to] = block.timestamp;

    return _mintAmount;
  }

This allows an attack path where an attacker can front-run other users initial deposit with a minimum of 1 wei mint deposit to prevent them for depositing first until full cooldown has passed. Attacker can repeat with more front-runs when the user tries to deposit again and make their tx revert.

Proof of Concept

1. User A deposits 10 WETH to a vault via multiTokenDepositFor() -> _depositAndMint()
2. Attacker front-runs User A to deposit for their address _depositFor with a minimum deposit that mints 1 wei portfolio token
3. User A's TX executes
4. User A now has to wait for the full cooldown to deposit their liquidity
5. Attacker can potentially repeat until infinity with all users, since he can simply front-run the users deposit TXs with a 1 wei mint deposit and the users tx will revert due to the cooldown applied
6. As long as attacker front-runs the deposits he can DOS all incoming deposits

Recommendation

Consider to set the cooldown for the actual depositor aka msg.sender, not the user who the depositor deposited to with the _depositFor param

[0xfuje]

Same with withdrawals

[langnavina97]

The cooldown period for very small investments is minimal. This design ensures that small, malicious deposits do not significantly impact the overall functionality and usability of the vaults.

@0xfuje

[0xfuje]

Appreciate the comment @langnavina97. The problem is that any cooldown period initiated will make the initial user's TX revert, because the attacker can front-run every deposit and withdraw to start the cooldown therefore blocking all deposits and withdrawals.

[0xfuje]

To be more specific, the exploit applies for withdrawals only, but I did make this comment before to highlight it: https://github.com/hats-finance/Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77/issues/12#issuecomment-2181037021, just my initial assumption was incorrect in that it also applies for deposits

[0xfuje]

Hey @langnavina97, please let me know why did you mark this as invalid

[langnavina97]

Minting 1 wei portfolio token won't be possible, it will fail here: https://github.com/hats-finance/Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77/blob/aa47c9ff85bcc2bede62978c3895668b549da125/contracts/core/calculations/VaultCalculations.sol#L33-L34

[0xfuje]

Thanks that's true, however I still think this does not solve the root of the issue since anyone can still block withdrawals as long they mint the minimum amount for the withdrawer. It can be a 10000x+ difference between the intended withdrawn amount and the amount of capital needed to block the deposit. Although I understand why you would mark it as invalid because attacker has to spend more than 1 wei of portfolio tokens

[kakarottosama]

for what I understand, having a minimum amount is kinda protection, and they already decide this 'attack' vector as 'acceptable'. If some attacker want to delay the transaction, they basically need to donate at least that minimum amount. Simply, the delayed user will earn the minimum deposit, which imo, the loss will be on the attacker side in the long run.

[burhankhaja]

@kakarottosama since i got the duplicate of this issue , i believe @0xfuje is right because Imagine:

In a situation, where portfolio is performing bad, imagine a whale investor is attacked, and his withdrawal txns are blocked, Dont you think his looses can skyrocket to millions of dollars.

Since, it is none of my business but as a auditor myself, i know the pain of submitting issues with blood n sweat while getting invalid