hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

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

First depositor can steal 2nd depositors money due to improper slippage protection #16

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: @@deadrosesxyz Twitter username: @deadrosesxyz Submission hash (on-chain): 0x797e6d7da8062bd6709f641c1525ba651d84eb2a8bfdfa372fd040686df84e71 Severity: medium

Description: Description\ First depositor can steal 2nd depositors money due to improper slippage protection

Attack Scenario\ The slippage protection is improper as it does not verify the received tokens have the needed value.

  1. Let's imagine vault is USDT/USDC
  2. Attacker deposits $1000 of each and is minted initialLiquidity
  3. Innocent user wants to also deposit $1000 of each and puts slippage protection as 0.98 * initialLiquidity
  4. Attacker front-runs and withdraws all funds from portfolio. They then do a deposit for 1 USDC and 1000 USDT and get minted initialLiquidity.
  5. Victim's tx executes and they're minted initialLiquidity.
  6. Even though Victim deposited 2x more, they get the same tokens as attacker. Attacker can then withdraw and profit 500 USDC

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

deadrosesxyz commented 1 week ago

https://www.bscscan.com/tx/0x08ea633fbf4ae27d01dd4f848770adee6d63bc6e9a523cf877cda69a0568f0d2

langnavina97 commented 1 week ago

The current design ensures that only the token amounts actually used are transferred, and the corresponding tokens are minted based on these amounts. The initial deposit sets the precedent for the value and weight distribution within the vault. Subsequent deposits are minted based on the ratio of tokens added to the vault at that time.

The potential for front-running as described is mitigated because each deposit transaction is handled independently, and tokens are minted proportionally to the token amounts deposited.

@deadrosesxyz

deadrosesxyz commented 1 week ago

Yes, I was missing context at time of submission. Issue can be closed as invalid.