hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

Attacker can steal all portfolio assets due to silent overflow #21

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @@deadrosesxyz Twitter username: @deadrosesxyz Submission hash (on-chain): 0xd9ac8a6899d0a885ac2141fab997cca1d146f1930e67629f056d635a731cc218 Severity: high

Description: Description\ Attacker can steal all portfolio assets due to silent overflow

Attack Scenario\ When depositing funds in the contract, depositAmounts is passed as a uint256 array.

  function _multiTokenDepositWithPermit(
    address _depositFor,
    uint256[] calldata depositAmounts,
    uint256 _minMintAmount,
    IAllowanceTransfer.PermitBatch calldata _permit,
    bytes calldata _signature
  ) internal virtual {

However, within _transferToVaultWithPermit it is truncated to uint160.

  function _transferToVaultWithPermit(
    address _from,
    address _token,
    uint256 _depositAmount
  ) internal {
    permit2.transferFrom(_from, vault, uint160(_depositAmount), _token);
  }

This allows for the following attack

  1. User deposits amount of uint160.max + 1
  2. When calculating the ratio, the value of uint160.max + 1 is used, hence the user will obtain pretty much all liquidity
  3. When transfer within permit2 is called, value is truncated to just 1 wei.
  4. User can then withdraw all funds from the contract.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

langnavina97 commented 4 months ago

No changes are required. Our protocol calculates the actual ratio based on the deposited amounts. This ensures that if an amount overflows uint160 and gets truncated to 1 wei, the resulting ratio would be close to 0, causing the deposit to fail. The protocol's design inherently protects against this issue by using actual ratios, which prevents the loss of funds as described in the attack scenario.

@deadrosesxyz

deadrosesxyz commented 4 months ago

Correct, I was missing full context at the time of submission. This issue can be closed as invalid.