Emmanuel - Exiting the market as a trader results in a deficit in collateral balance.

[sherlock-admin]

Emmanuel

medium

Exiting the market as a trader results in a deficit in collateral balance.

Summary

It is expected that a trader that closes all his position and withdraws all his collateral will have a collateral balance of 0, but this is not so due to the delayed settlement of position fees

Vulnerability Detail

When a user updates his positions, the position fees and keeper fees does not get deducted immediately, rather it gets deducted during the next settlement.
If a trader decides to exit a market by closing all his positions and withdrawing all his collateral, the fees will be deducted during the next settlement, so his collateral balance becomes negative.
If trader does not enter the market again(very possible if trader is a contract controlled by an EOA), collateral balance of that account remains negative, so the assumption that account MUST pay back during the next settlement holds false.
When the fees get withdrawn by protocol, other users' collateral pay for the fees that the malicious trader ought to have paid.

Similarly, during liquidations, fees are deducted from the liquidated account's collateral balance during the next settlement, instead of it to be deducted from the liquidationFee that liquidator will receive. This is unfair to the liquidated account because now, they are paying liquidationFee+keeperFee+positionFee, instead of them to pay just liquidationFee.

Impact

• Traders that permanently exit Market will not pay position fees, so other users are forced to pay it.
• Accounts being liquidated will pay liquidationFee+keeperFee+positionFee instead of just liquidationFee

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L345
https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/Market.sol#L440
https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial/contracts/types/Local.sol#L72

Tool used

Manual Review

Recommendation

When updating position, users should only be able to withdraw up to collateralBalance-(newOrder.fee+newOrder.keeper), so that when fees get debited during next settlement, collateralBalance will be 0 and not negative.
Also, during liquidation, liquidator should receive liquidatioFee-(newOrder.fee+newOrder.keeper)

[sherlock-admin]

2 comment(s) were left on this issue during the judging contest.

141345 commented:

m

panprog commented:

invalid because if user tries to do this, his collateral is removed immediately but position still stays the same and the account will be insolvent thus update will revert. As for liquidation fee - this works as intended, user pays his due fees for position closure on his behalf and liquidation fee for liquidation itself.

[arjun-io]

A user cannot fully withdraw their collateral in the same update where they set their position to 0 as this will put them below the minimum maintenance requirement. They will need to wait to withdraw collateral until after the position change has settled, at which time position fees will be properly deducted.

[Emedudu]

Escalate

Deducting of keeper fees separately from liquidationFee is a problem because in case liquidatorFee causes position's balance to be 0, deducting the fees during next settlement will result in deficit

[sherlock-admin2]

Escalate

Deducting of keeper fees separately from liquidationFee is a problem because in case liquidatorFee causes position's balance to be 0, deducting the fees during next settlement will result in deficit

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.

[panprog]

Escalate

There is no problem as it's expected behavior. Liquidation fee alone can also put account into deficit, it's not like fees are something that is the only way to put account into deficit. Moreover, even if account is not liquidated, its collateral can become negative just due to price movement.
And position closure fees on top of liquidation fee is expected behavior, user's position is forcedly closed on his behalf, so both liquidation and position closure fees are taken.
So this issue should still be invalid.

[sherlock-admin2]

Escalate

There is no problem as it's expected behavior. Liquidation fee alone can also put account into deficit, it's not like fees are something that is the only way to put account into deficit. Moreover, even if account is not liquidated, its collateral can become negative just due to price movement.
And position closure fees on top of liquidation fee is expected behavior, user's position is forcedly closed on his behalf, so both liquidation and position closure fees are taken.
So this issue should still be invalid.

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.

[Emedudu]

There is no problem as it's expected behavior

If a position is in a deficit, it implies that other users will be forced to cover the debt accrued by the account.

Additionally, keep in mind that a user can self-liquidate. They can open multiple positions with different accounts and subsequently liquidate them, resulting in several accounts carrying a negative collateral balance (this debt will be paid by other users).

[panprog]

If a position is in a deficit, it implies that other users will be forced to cover the debt accrued by the account.

Account deficit by itself is an expected protocol functionality: collateral is intentionally signed and can be negative. So this if by design.

This report states the following issues:

• User can close and withdraw with fees subtracted only after settlement, making account in deficit. This is incorrect: user must keep position maintenance collateral until position is settled, so collateral can not be 0 while position closure is still not settled.
• When being liquidated, users pay liquidation fee + keeper fee + position closure fee instead of liquidation fee alone. This is by design and expected behavior: user pays for the position closure on his behalf. So this is not an issue either.

Since both issues stated in the report are invalid, the report itself should be invalid.

Stated impacts are

• Traders that permanently exit Market will not pay position fees, so other users are forced to pay it.
• Accounts being liquidated will pay liquidationFee+keeperFee+positionFee instead of just liquidationFee

While accounts in deficit might be a problem for the other users, this report doesn't talk about it specifically, only in relation to fees which might have to be paid by the other users (which is incorrect as explained above). And the issue stated is fees, not the account deficit by itself.

Additionally, keep in mind that a user can self-liquidate. They can open multiple positions with different accounts and subsequently liquidate them, resulting in several accounts carrying a negative collateral balance (this debt will be paid by other users).

This is not described in the report at all: self-liquidation is not an easy operation to do and an issue by itself. Even when user can self-liquidate, minMaintenance collateral >= minLiquidationFee restriction (when setting market risk parameters) ensures that the user account is not in deficit even if intentionally self-liquidated.

[Emedudu]

The core issue this report is pointing out is that deducting of keeper fees during next settlement could lead to a deficit in collateral balance.
The report explained two reasons why this behaviour could be a problem:

1.) User can close and withdraw with fees subtracted only after settlement, making account in deficit. This is incorrect: user must keep position maintenance collateral until position is settled, so collateral can not be 0 while position closure is still not settled.

This scenario has been correctly invalidated by the judges and sponsor

2.)When being liquidated, users pay liquidation fee + keeper fee + position closure fee instead of liquidation fee alone. This is by design and expected behavior: user pays for the position closure on his behalf. So this is not an issue either.

This issue also explains that users pay liquidation fee + keeper fee + position fee due to the delayed deduction of fees.
This is a PROBLEM because it can lead to a deficit in collateral balance, which means other users have to pay for the debt.

So there can be a scenario where a trader uses a contract(or many contracts) to open positions, and when it gets liquidated, the balance becomes negative(due to delayed fee deduction), and the trader abandons the account.
This means that other users will pay for the debt that the trader has caused, leading to loss of funds for other users.

[141345]

The problem is not about keeper fees separated from liquidationFee. The focus is deficit here. But liquidation itself can create deficit as well, as panprog points out.

Most protocols face profit and loss, limited deficit should not be vulnerability.

Keeper fee combined with liquidationFee could be future design consideration.

Low severity seems more appropriate.

[Emedudu]

The focus is deficit here. But liquidation itself can create deficit as well, as panprog points out.

Yes. Liquidation causing a deficit is also a problem, and has been reported here.

Most protocols face profit and loss, limited deficit should not be vulnerability.

Please note that this loss will be paid by some other unlucky users(specifically, the last set of users to withdraw their collateral).
It is true that a deficit of positionFees amount may not mean much, however, this will easily add up especially when done deliberately:

• Attacker opens numerous little positions with contract accounts, making sure that they are almost liquidatable
• Attacker monitors and liquidates the positions when they get liquidatable.
• position fees causes each of those liquidated positions' balance to become negative
• Attacker can perform this multiple times since he has nothing to lose

Other users will be forced to pay this created debt, and this is the main reason why I believe this is an issue->loss of funds of other users

[141345]

If the attacker intentionally create deficits, multiple positions need to be opened, the cost of the attack is quite high. Seems there is no profit for the attacker.

[Emedudu]

Even if the malicious user does not profit, they lose nothing as well, while causing some other users to lose.

Additionally, the malicious user can profit if, due to PnL, their collateral balance falls below the minLiquidationFee. In this case, they will gain minLiquidationFee - collateralBalance. Then, deducting positionFees in the next settlement will create a greater deficit.

[hrishibhat]

Result:
Low
Unique
After further discussion and based on all the comments above, I agree with @panprog and @141345's comment as the main issue of account deficit is expected.

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• panprog: accepted
• Emedudu: rejected