code-423n4 / 2023-09-goodentry-mitigation-findings

0 stars 0 forks source link

GeVault LP calculations do not use scaling and are vulnerable to deposit attacks #50

Closed c4-submissions closed 12 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/GoodEntry-io/ge/blob/c7c7de57902e11e66c8186d93c5bb511b53a45b8/contracts/GeVault.sol#L290

Vulnerability details

Impact

Currently, getTVL counts all token balances in GeVault, and attackers can manipulate getTVL to implement deposit attacks. Secondly, since the calculation of LP does not use scaling, because there is a precision error in the division, the cost for the attacker to implement the attack is very low.

Proof of Concept

Let me give you an example with specific numbers:

  1. Attacker deposit: tSupply = vaultValueX8 = 0, valueX8 = 1e-10, liquidity = 1
  2. Attacker frontrun to transfer: valueX8 += 1e8 - 1e-10 + 1
  3. User deposit: tSupply = 1, vaultValueX8 = 1e8 + 1, valueX8 = 2e8, liquidity = 1
  4. Attacker withdraw: tSupply = 2, vaultValueX8 = 3e8 + 1, valueX8 = 1.5e8, profit = 0.5e8

Tools Used

Manual review

Recommended Mitigation Steps

  1. Use high precision for scaling to increase the attacker’s cost of attack
  2. Mint a certain amount of tokens during initialization to avoid deposit attacks

Assessed type

ERC4626

c4-judge commented 12 months ago

gzeon-c4 marked the issue as duplicate of #55

c4-judge commented 12 months ago

gzeon-c4 marked the issue as not a duplicate

c4-judge commented 12 months ago

gzeon-c4 marked the issue as unsatisfactory: Out of scope

gzeon-c4 commented 12 months ago

Same as M-04 where the team decided not to fix.