sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

cergyk - Leveraged trader with small collateral can create a riskless position until settlement #104

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

cergyk

high

Leveraged trader with small collateral can create a riskless position until settlement

Summary

A highly leveraged trader (around max leverage) with small collateral can autoliquidate position to create a riskless position

Vulnerability Detail

As we can see in closeTake function: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/product/Product.sol#L246-L256

And in liquidate: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/collateral/Collateral.sol#L128-L129

We can deduce that a highly leveraged trader, with very small collateral (0.5 DSU) can create a riskless position lasting until next settlement by auto liquidating.

Example: Maintenance factor is 2% Liquidation fee is 5% Taker fee is 0.1% Funding fee is 0%

Alice has a 50x leverage taker position opened as authorized per maintenance invariant:

Alice collateral: 0.5 DSU Alice taker position: notional of 25 DSU

To create the riskless position Alice does in the same tx:

For the rest of the oracle version Alice has a 25 DSU position with 0 collateral. Which means that in the end of the period:

If Alice has multiple such positions on multiple accounts, considerable amounts can be stolen this way from the protocol.

Impact

Alice obtains a riskless position, inducing potential shortfall on the protocol

Code Snippet

Tool used

Manual Review

Recommendation

Do not add positive Pnl to the collateral of an account if it does not have any collateral (to enforce the invariant: A position can not be without risk)

SergeKireev commented 1 year ago

Escalate for 10 USDC

It seems this issue has been deemed non-valid because of the minCollateral requirement.

However in current setup on arbitrum (funding fees == 0), it is actually easy to create two opposite positions (one long and one short) with exactly MIN_COLLATERAL, such that the sum of the values stays constant. One of these is very likely to end up close to 10% of MIN_COLLATERAL (since when one position increases collateral, the other decreases collateral).

When that happens, the attacker can use the technique described in this report to create a riskless position until next settlement and effectively steal from other users of the market

sherlock-admin commented 1 year ago

Escalate for 10 USDC

It seems this issue has been deemed non-valid because of the minCollateral requirement.

However in current setup on arbitrum (funding fees == 0), it is actually easy to create two opposite positions (one long and one short) with exactly MIN_COLLATERAL, such that the sum of the values stays constant. One of these is very likely to end up close to 10% of MIN_COLLATERAL (since when one position increases collateral, the other decreases collateral).

When that happens, the attacker can use the technique described in this report to create a riskless position until next settlement and effectively steal from other users of the market

You've created a valid escalation for 10 USDC!

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.

KenzoAgada commented 1 year ago

@arjun-io can you please read and share your perspective?

arjun-io commented 1 year ago

This does appear to be possible, although the work required here to get the position at minCollateral is quite a bit. The attacker will be paying 4x open/close fees in order to get their position into this state - and the most efficient way to get to this sub-minCollateral position is by opening opening highly levered positions which will increase the fee

I would consider this a valid medium

jacksanford1 commented 1 year ago

Since protocol team validates it as possible, seems like this should be a valid Medium. Funds are risk seems potentially too small, but this type of attack could cause a chain reaction once everyone else realizes they are losing small amounts of funds quickly.

jacksanford1 commented 1 year ago

Result: Medium Unique Valid Medium based on Arjun's last comment. This issue is very similar to #103 but I could not make quite enough of a case to duplicate them.

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status: