sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

mstpr-brainbot - partyB can leverage emergency mode for quick profits #192

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

mstpr-brainbot

medium

partyB can leverage emergency mode for quick profits

Summary

A partyB can exploit the system by requesting the activation of emergency mode when they have an open position that partyA is reluctant to close. In the interim, partyB can take advantage of any open quotes with an expected open price below the current market rate. Once partyB is granted emergency status, they can immediately close these trades for an quick profit.

Vulnerability Detail

Suppose Alice, acting as partyB, has an ongoing position with a counterparty, partyA, who has refrained from closing the position. Sensing the need for an intervention, Alice appeals to the protocol for the activation of emergency mode. However, just before the transaction granting emergency status is processed, Alice conducts a quick sweep for any open quotes where the proposed open price is less than the current market rate. Alice stumbles upon a quote where partyA offers a LONG position on 100 units of ETH at an expected open price of $2000 each. Given that the prevailing market price for ETH stands at $2010, Alice promptly takes advantage of the opportunity by opening that quote. As soon as Alice is granted emergency status, she can close the trade and immediately pocket a neat profit of $1000 [(2010-2000) x 100] within quick operations.

Impact

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/PartyB/PartyBFacetImpl.sol#L309-L321

Tool used

Manual Review

Recommendation

Make the emergency status specific for a partyA instead of granting the partyB for its all positions

mstpr commented 1 year ago

Escalate

Above scenario can be easily leveraged by a partyB that has emergency status. Closing trades in seconds is not an intended behaviour in the current design so that's why this is a valid medium.

sherlock-admin2 commented 1 year ago

Escalate

Above scenario can be easily leveraged by a partyB that has emergency status. Closing trades in seconds is not an intended behaviour in the current design so that's why this is a valid medium.

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.

MoonKnightDev commented 1 year ago

Party Bs are permissionless roles in this system. Moreover, the emergency status is intended for situations where Party B wants to close all positions, not just the positions of one user. Additionally, the admin has the authority to set the emergency status for Party Bs.

mstpr commented 1 year ago

@MoonKnightDev What I am arguing in there is not about partyB closing all the existed positions. PartyB can lock and close any position that is in profit.

Assume that that partyB has 2 trades going on with 2 different partyA's and protocol team gave the partyB the emergency status. What's expected is that partyB will close both of these positions and protocol team will revoke the emergency status.

Now, this partyB can quickly scan the open trades, if partyB sees any trade that is profitable at the time of opening and immediately closing (Check the impact section for the detailed scenario) partyB can quickly open that quote and close it for quick profits. Thus, partyB is not only closing its existed 2 positions as intended but it also can open and close any quote that is profitable at the time which is not intended for any partyA.

MoonKnightDev commented 1 year ago

Yes, it's better to check Party B's status, ensuring they are not in emergency mode in the open position function.

hrishibhat commented 1 year ago

Result: Medium Unique Considering this a valid medium based on the above comments

sherlock-admin2 commented 1 year ago

Escalations have been resolved successfully!

Escalation status:

naveedinno commented 1 year ago

Fixed code PR link: https://github.com/SYMM-IO/symmio-core/pull/27