sherlock-audit / 2024-08-velar-artha-judging

6 stars 2 forks source link

KupiaSec - Funding fees should be calculated based on position volumes rather than utilization ratio #76

Closed sherlock-admin2 closed 1 week ago

sherlock-admin2 commented 1 week ago

KupiaSec

High

Funding fees should be calculated based on position volumes rather than utilization ratio

Summary

Funding fees for perp positions are calculated using utilization ratio of base/quote assets. This calculation is invalid since the protocol allows imbalance between base/quote assets in the pool.

Vulnerability Detail

In the protocol, funding fees are paid between long and shot perp traders. The basic design is that at any moment, if long perp trades happen more than short perp trades, long position openers pay short position openers the funding fee, and vice versa. This funding fees provides stability of the protocol and prevents loss of LPs and the protocol.

However, the funding fee calculation is executed in improper way, as shown below:

# How funding_long and funding_short are calculated

funding_long     : uint256 = self.funding_fee(
  borrowing_long, long_utilization,  short_utilization)
funding_short    : uint256 = self.funding_fee(
  borrowing_short, short_utilization,  long_utilization)

...

# funding_fee implementation

def funding_fee(base_fee: uint256, col1: uint256, col2: uint256) -> uint256:
  imb: uint256 = self.imbalance(col1, col2)
  if imb == 0: return 0
  else       : return self.check_fee(self.scale(base_fee, imb))

As shown in the code snippet, funding fees are calculated using long and short utilization of the pool. Thus, if long utilization > short utilization, long position openers pay funding fees for short position openers, and vice versa.

The problem is that the protocol allows imbalance between base and quote assets in the pool, for example, assuming 1 BTC = 50,000 USDC, the value of base/quote assets might be different like 1 BTC + 10,000 USDC. So as a result, even though long utilization > short utilization, actual notional long position size can be smaller than notional short size.

This vulnerability can cause significant loss to LP providers and the overall protocol because of reverse party paying funding fees.

Here's the scenario that loss can happen because of this vulnerability.

This logic allows more and more long position openers to open their positions and less incentivize short openers. At the end, it incurs significant losses to LP providers and to the protocol.

Impact

Loss for LP providers and the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-08-velar-artha/blob/18ef2d8dc0162aca79bd71710f08a3c18c94a36e/gl-sherlock/contracts/params.vy#L46-L49

Tool used

Manual Review

Recommendation

The funding fee calculation logic has to be modified so that it uses actual value of positions rather than the utilization ratio to determine which party pays the funding fees.

KupiaSecAdmin commented 1 week ago

Escalate

This issue deserves High.

sherlock-admin3 commented 1 week ago

Escalate

This issue deserves High.

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.

msheikhattari commented 1 week ago

This is a design choice. The market will equilibriate since if fees are high positions will close and LPs will enter until rates balance out.

KupiaSecAdmin commented 1 week ago

This is a design choice. The market will equilibriate since if fees are high positions will close and LPs will enter until rates balance out.

This design choice is absolutely dangerous as shown in the PoC above. Also the statement about this can't be issue because the market will equilibrate because LPs close positions is not true. It's only assumption, can't be a factor to invalidate this issue.

msheikhattari commented 1 week ago

Because of the nature of calc_burn it's also unlikely that the pool would ever become so imbalanced. It would require LPs to persistently provide liquidity in a single token, and for very few LPs to exit the pool for a long duration of time.

The alternative choice of using position values across both sides of the pool has dangers as well and requires a total redesign of this facet of the protocol