Skip to content
This repository has been archived by the owner on Jan 7, 2024. It is now read-only.

AkshaySrivastav - Liquidators can prevent users from making their positions healthy during an unpause #336

Open
sherlock-admin opened this issue Jul 3, 2023 · 9 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin
Copy link
Contributor

AkshaySrivastav

medium

Liquidators can prevent users from making their positions healthy during an unpause

Summary

The Symmetrical protocol has various paused states in which different operations are paused. The protocol operations can be unpaused by privileged accounts. But when this unpause happens the liquidators can frontrun and liquidate user positions before those users get a chance to make their positions healthy.

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/control/ControlFacet.sol#L242-L295

Vulnerability Detail

The privileged addresses can pause the symmetrical protocol fully/partially using the pausability functions in ControlFacet. Once the protocol is paused all these operations cannot be done by users:

  • deposit
  • withdraw
  • allocate
  • deallocate
  • sendQuote
  • lockQuote
  • openPositions
  • liquidations
  • etc

Though, real prices from oracles will surely move up or down during this paused period. If the oracle prices go down, the users won't be allowed to allocate more collateral to their positions or close their positions. Hence their positions will get under-collateralized (based upon real prices).

Once the protocol is unpaused the liquidators can front-run most users and liquidate their positions. Most users will not get a chance to make their position healthy.

This results in loss of funds for the users.

Ref: sherlock-audit/2023-03-notional-judging#203

Impact

By front-running any collateral allocation or position closure of a legitimate user which became under-collateralized during the paused state, the liquidator can unfairly liquidate user positions and collect liquidation profit as soon as the protocol is unpaused. This causes loss of funds to the user.

Also, on unpause, it is unlikely that any human users will be able to secure their positions before MEV/liquidation bots capture the available profit. Hence the loss is certain.

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/control/ControlFacet.sol#L242-L295

Tool used

Manual Review

Recommendation

Consider adding a grace period after unpausing during which liquidation remains blocked to allow users to avoid unfair liquidation by closing their positions or allocating additional collateral.
The protocol team can also think of any other way which mitigates the unfair liquidations of users.

@github-actions github-actions bot added the Medium A valid Medium severity issue label Jul 10, 2023
@MoonKnightDev MoonKnightDev added the Sponsor Disputed The sponsor disputed this issue's validity label Jul 16, 2023
@MoonKnightDev
Copy link

These pauses are designed for emergency scenarios. When we do initiate a pause, we should formulate specific policies before it's unpaused. As per your example, one strategy could be to temporarily pause liquidations, providing users with an opportunity to distance themselves from a potential liquidation event. This is just one example; any policy can be considered. Therefore, we don't view this as a bug, rather it is an integral part of our system.

@ctf-sec ctf-sec closed this as completed Jul 20, 2023
@sherlock-admin sherlock-admin added Non-Reward This issue will not receive a payout and removed Medium A valid Medium severity issue labels Jul 26, 2023
@akshaysrivastav
Copy link

Escalate for 10 USDC

I think this issue should be considered as valid. As per the sponsor comment above it is evident that the current protocol implementation does not contain any safeguard mechanism against this issue and an additional code change (temporary policies) will be needed to prevent users from the loss of funds due to the issue.

The report clearly shows how a protocol owner action (pause) will result in unfair liquidations causing loss of funds to users.

For reference, similar issues were considered valid in the recent Notional V3 and Blueberry contests and was accepted by Sherlock. Maintaining a consistent valid/invalid classification standard will be ideal here.

@sherlock-admin2
Copy link

Escalate for 10 USDC

I think this issue should be considered as valid. As per the sponsor comment above it is evident that the current protocol implementation does not contain any safeguard mechanism against this issue and an additional code change (temporary policies) will be needed to prevent users from the loss of funds due to the issue.

The report clearly shows how a protocol owner action (pause) will result in unfair liquidations causing loss of funds to users.

For reference, similar issues were considered valid in the recent Notional V3 and Blueberry contests and was accepted by Sherlock. Maintaining a consistent valid/invalid classification standard will be ideal here.

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.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Jul 27, 2023
@ctf-sec
Copy link
Collaborator

ctf-sec commented Jul 28, 2023

These pauses are designed for emergency scenarios. When we do initiate a pause, we should formulate specific policies before it's unpaused. As per your example, one strategy could be to temporarily pause liquidations, providing users with an opportunity to distance themselves from a potential liquidation event. This is just one example; any policy can be considered. Therefore, we don't view this as a bug, rather it is an integral part of our system.

I have to side with sponsor on this one

there is no way to prevent frontrunning pause or unpause even adding grace period..

In fact, there are a lot of similar finding in recent contest about unpause / pause,

they just end up "won't fix"

sherlock-audit/2023-03-notional-judging#203

The blueberry issue talks about repay is paused when liquidation is active, which is not the same as frontrunning pause or unpause

@akshaysrivastav
Copy link

Hey @ctf-sec I totally respect your decision but just want to state my points in a much clearer way as there seems to be a misunderstanding.

  • The issue is not about frontrunning the admin's pause/unpause txn,
  • The issue is about, when admin pauses the operations the users are prevented from changing their positions, these positions can become unheathy during paused state, and as soon as admin unpauses the protocol, mev actors can liquidate the users before those users get a chance to save their positions. In short, a protocol owner action (pause) will result in unfair liquidations for users causing loss of funds.
  • As the sponsors already confirmed that they will need to introduce temporary policies to mitigate this issue. This confirms two things:
    • The bug is real, loss of funds is real, and new code changes will be needed for mitigation.
    • Since sponsors will be forced to implement new policies, this issue will not technically end up in the "won't fix" category.
  • Agree on blueberry, actual similar issues to this one are NotionalV3:203 and Perennial:190, both were considered as valid by Sherlock.

@ctf-sec
Copy link
Collaborator

ctf-sec commented Jul 28, 2023

Hi Akshay,

I think you are trying to refer to this issue?

sherlock-audit/2023-04-blueberry-judging#117

looks like exact the same issue,

I think a valid medium is ok

@hrishibhat hrishibhat added Medium A valid Medium severity issue and removed Non-Reward This issue will not receive a payout labels Aug 19, 2023
@sherlock-admin sherlock-admin added Reward A payout will be made for this issue labels Aug 19, 2023
@hrishibhat
Copy link

Result:
Medium
Unique
Considering this a valid medium based on historical decisions and the issue is still valid from smart contract POV.

Also, however, given that this clearly can be design decision for some protocols, agree that there should be a better rule around these protocol-pausing situations.

@hrishibhat hrishibhat reopened this Aug 19, 2023
@sherlock-admin2 sherlock-admin2 removed the Escalated This issue contains a pending escalation label Aug 19, 2023
@sherlock-admin sherlock-admin added the Escalation Resolved This issue's escalations have been approved/rejected label Aug 19, 2023
@sherlock-admin2
Copy link

Escalations have been resolved successfully!

Escalation status:

@hrishibhat
Copy link

Additionally, #281 can be a valid duplicate of this issue

@hrishibhat hrishibhat added the Has Duplicates A valid issue with 1+ other issues describing the same vulnerability label Aug 19, 2023
@naveedinno naveedinno added the Won't Fix The sponsor confirmed this issue will not be fixed label Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Has Duplicates A valid issue with 1+ other issues describing the same vulnerability Medium A valid Medium severity issue Reward A payout will be made for this issue Sponsor Disputed The sponsor disputed this issue's validity Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

7 participants