bughuntoor - First depositor could DoS the pool

[sherlock-admin2]

bughuntoor

Medium

First depositor could DoS the pool

Summary

First depositor could DoS the pool

Vulnerability Detail

Currently, when adding liquidity to a pool, the way LP tokens are calculated is the following:

1. If LP total supply is 0, mint LP tokens equivalent the mint value
2. If LP total supply is not 0, mint LP tokens equivalent to mintValue * lpSupply / poolValue

def f(mv: uint256, pv: uint256, ts: uint256) -> uint256:
  if ts == 0: return mv
  else      : return (mv * ts) / pv    # audit -> will revert if pv == 0

However, this opens up a problem where the first user can deposit a dust amount in the pool which has a value of just 1 wei and if the price before the next user deposits, the pool value will round down to 0. Then any subsequent attempts to add liquidity will fail, due to division by 0.

Unless the price goes back up, the pool will be DoS'd.

Impact

DoS

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/main/gl-sherlock/contracts/pools.vy#L178

Tool used

Manual Review

Recommendation

Add a minimum liquidity requirement.

[msheikhattari]

Escalate

DoS has two separate scores on which it can become an issue:

1. The issue causes locking of funds for users for more than a week (overrided to 4 hr)
2. The issue impacts the availability of time-sensitive functions (cutoff functions are not considered time-sensitive). If at least one of these are describing the case, the issue can be a Medium. If both apply, the issue can be considered of High severity. Additional constraints related to the issue may decrease its severity accordingly.
   Griefing for gas (frontrunning a transaction to fail, even if can be done perpetually) is considered a DoS of a single block, hence only if the function is clearly time-sensitive, it can be a Medium severity issue.

Low at best. Per the severity guidelines, this is not DoS since no user funds are locked and no sensitive functionality is impacted (ongoing positions/LPs are not impacted). Additionally, this both assumes that no other LPs make any deposits within the same block as the attacker (as the price would be equivalent), and that the price is monotonically decreasing after the attack was initiated. Not only is it low impact, but also low likelihood.

[sherlock-admin3]

Escalate

DoS has two separate scores on which it can become an issue:

1. The issue causes locking of funds for users for more than a week (overrided to 4 hr)
2. The issue impacts the availability of time-sensitive functions (cutoff functions are not considered time-sensitive). If at least one of these are describing the case, the issue can be a Medium. If both apply, the issue can be considered of High severity. Additional constraints related to the issue may decrease its severity accordingly.
   Griefing for gas (frontrunning a transaction to fail, even if can be done perpetually) is considered a DoS of a single block, hence only if the function is clearly time-sensitive, it can be a Medium severity issue.

Low at best. Per the severity guidelines, this is not DoS since no user funds are locked and no sensitive functionality is impacted (ongoing positions/LPs are not impacted). Additionally, this both assumes that no other LPs make any deposits within the same block as the attacker (as the price would be equivalent), and that the price is monotonically decreasing after the attack was initiated. Not only is it low impact, but also low likelihood.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[WangSecurity]

While indeed this doesn't qualify the DOS requirements, this issue can still result in the functionality of the protocol being blocked and the users wouldn't be able to use this pool. I agree it's low likelihood, but it's still possible.

@spacegliderrrr since this issue relies on the precision loss, it requires a POC, can you make one? And also how low should be the price so it leads to rounding down?

[spacegliderrrr]

Price doesn't really matter - it's just needed to deposit little enough tokens that they're worth 1 wei of quote token. So if for example the pair is WETH/ USDC, a user would need to deposit ~4e8 wei WETH (considering price of $2,500).

As for the PoC, because the code is written in Vyper and Foundry does not support it, I cannot provide a PoC.

[WangSecurity]

In that case, could you make a very detailed attack path, I see you provided an example of the price that would cause an issue, but still would like a very detailed attack path.

[WangSecurity]

One important thing to consider is the following question:

We will report issues where the core protocol functionality is inaccessible for at least 7 days. Would you like to override this value?
Yes, 4 hours

So, I see that DOS rules say that the funds should be locked for a week. But, the question is about core protocol functionality being inaccessible. The protocol specified they want to have issues about core protocol functionality being inaccessible for 4 hours.

This finding, indeed doesn't lead to locking funds or blocking time-sensitive functions, but it can lead to the core protocol functionality being inaccessible for 4 hours. I see that it's low likelihood, but the likelihood is not considered when defining the severity. Hence, even though for this finding there have to be no other depositors in the next block or the price being lower for the next 4 hours, this can happen. Thus, medium severity for this issue is appropriate. Planning to reject the escalation and leave the issue as it is.

[msheikhattari]

This issue doesn't impact ongoing operations, so its similar to frontrunning of initializers. No irreversible damage or loss of funds occur.

Core protocol functionality being inaccessible should have some ramifications like lock of funds or broken time-sensitive functionality (like withdrawals). No funds are in the pool when this issue is taking place.

[msheikhattari]

In any case this issue does need a PoC - per the severity criteria all issues related to precision loss require one.

[WangSecurity]

@spacegliderrrr could you make a detailed numerical POC, showcasing the precision loss and DOS.

Core protocol functionality being inaccessible should have some ramifications like lock of funds or broken time-sensitive functionality (like withdrawals). No funds are in the pool when this issue is taking place

As I've said previously, this still impacts the core protocol functionality, as the users cannot deposit into the pool, and this very well could last for more than 4 hours. Hence, this is sufficient for medium severity as the core protocol functionality is inaccessible for more than 4 hours.

[spacegliderrrr]

1. Market pair used is WETH/USDT. Current WETH price $2,500.
2. User is the first depositor in the pool, depositing 4e8 WETH. Based on the lines of code below, the user is minted 1 lp token.:
   amt0 = 4e8 * 2500e18 / 1e18 = 10000e8 = 1e12
lowered = 1e12 / 1e12 = 1

@external
@pure
def base_to_quote(tokens: uint256, ctx: Ctx) -> uint256:
  lifted : Tokens  = self.lift(Tokens({base: tokens, quote: ctx.price}), ctx)
  amt0   : uint256 = self.to_amount(lifted.quote, lifted.base, self.one(ctx))
  lowered: Tokens  = self.lower(Tokens({base: 0, quote: amt0}), ctx)
  return lowered.quote

3. Next block comes and WETH price drops to $2,499.
4. A user now attempts to deposit. Since LP tokens minted are respective to the current pool value, contract calculates pool value, using the same formula above
   amt0 = 4e8 * 2499e18 / 1e18 = 0.9996e12
lowered = 0.9996e12 / 1e12 = 0
5. User's LP tokens are calculated using the following formula. As the pool's value is 0, a division by 0 is attempted, which forces the tx to revert.

def f(mv: uint256, pv: uint256, ts: uint256) -> uint256:
  if ts == 0: return mv
  else      : return (mv * ts) / pv

6. Pool is DoS'd until ETH price goes above $2,500 again.

[WangSecurity]

As far as I can tell, the POC is correct and this issue will indeed happen. As was said previously, planning to reject the escalation and leave the issue as it is.

[WangSecurity]

Result:
Medium
Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• msheikhattari: rejected