PartyB unable to settle their hedges with their broker during emergency close

Summary

PartyB is unable to settle its hedges with its broker during an emergency close because PartyA could execute the emergencyClosePosition immediately after the protocol marks the symbol as invalid instead of having PartyB execute it. As a result, the following issues will occur, leading to loss of assets:

Break the workflow of the hedgers, and they will not be able to close their countertrade on their broker platform before the order is closed on-chain, disrupting the hedger’s ability to manage their countertrade effectively and their hedging strategy.
The lack of control over the timing of the close means hedgers cannot ensure that their broker-side positions are closed appropriately, increasing the risk of slippage or other adverse trading conditions

Vulnerability Detail

For most hedgers (PartyB) in SYMM IO, when they initiate a trade, they directly open a direct countertrade on their chosen broker platform (for instance, Binance) before filling the order on-chain.

When the hedgers fill a close request, they perform the opposite actions. Upon receiving a request from Party A to close a position at a limit, the corresponding hedger should either submit this request to a broker or continuously monitor the price. When the price reaches the desired level, or the close request from the broker's side is fulfilled, the hedger should proceed with the closure. The workflow of hedgers is clearly documented in SYMM's documentation here.

CloseLimit drawio

In the previous codebase, only the hedgers can initiate an emergency close. Following is the previous implementation of emergencyClosePosition, which implements a whenEmergencyMode(msg.sender) modifier. Within the whenEmergencyMode modifier, it will ensure only the callers can execute an emergency close of a position.

function emergencyClosePosition(
    uint256 quoteId,
    PairUpnlAndPriceSig memory upnlSig
)
    external
    whenNotPartyBActionsPaused
    onlyPartyBOfQuote(quoteId)
    whenEmergencyMode(msg.sender)
    notLiquidated(quoteId)
{

modifier whenEmergencyMode(address partyB) {
    require(
        GlobalAppStorage.layout().emergencyMode ||
            GlobalAppStorage.layout().partyBEmergencyStatus[partyB],
        "Pausable: It isn't emergency mode"
    );
    _;
}

In addition, per the documentation shared by the protocol team during the audit period, the documentation stated that if a symbol becomes invalid, the PartyB can emergency close all its positions. The purpose of the emergency function is to let PartyB close its positions without users' requests. It is not the opposite way where we let users (PartyA) close their positions without PartyB's request.

Explanation: From time to time, Binance delists some of its pairs. In the current version, we, as a platform, had to put partyBs in emergency mode to let them close their positions without users' requests. In the next version, if a symbol becomes invalid, partyB can emergency close all its positions.

However, in the new codebase, the emergencyClosePosition can be executed by anyone once the symbol has been marked as invalid by the protocol, even though the comment at Line 171 indicates that only PartyB is allowed to do so.

https://github.com/sherlock-audit/2024-06-symmetrical-update-2/blob/main/protocol-core/contracts/facets/PartyB/PartyBFacet.sol#L175

File: PartyBFacet.sol
170:    /**
171:     * @notice Allows Party B to emergency close a position for the specified quote.
172:     * @param quoteId The ID of the quote for which the position is emergency closed.
173:     * @param upnlSig The Muon signature containing the unrealized profit and loss (UPNL) and the closing price.
174:     */
175:    function emergencyClosePosition(
176:        uint256 quoteId,
177:        PairUpnlAndPriceSig memory upnlSig
178:    ) external whenNotPartyBActionsPaused onlyPartyBOfQuote(quoteId) notLiquidated(quoteId) {

https://github.com/sherlock-audit/2024-06-symmetrical-update-2/blob/main/protocol-core/contracts/facets/PartyB/PartyBFacetImpl.sol#L267

File: PartyBFacetImpl.sol
267:    function emergencyClosePosition(uint256 quoteId, PairUpnlAndPriceSig memory upnlSig) internal {
268:        AccountStorage.Layout storage accountLayout = AccountStorage.layout();
269:        Quote storage quote = QuoteStorage.layout().quotes[quoteId];
270:        Symbol memory symbol = SymbolStorage.layout().symbols[quote.symbolId];
271:        require(
272:            GlobalAppStorage.layout().emergencyMode || GlobalAppStorage.layout().partyBEmergencyStatus[quote.partyB] || !symbol.isValid,
273:            "PartyBFacet: Operation not allowed. Either emergency mode must be active, party B must be in emergency status, or the symbol must be delisted"
274:        );

As a result, PartyA could execute the emergencyClosePosition immediately after the protocol marks the symbol as invalid instead of having PartyB execute the emergency close by themselves.

Impact

The following issues will occur, leading to loss of assets:

Break the workflow of the hedgers, and they will not be able to close their countertrade on their broker platform before the order is closed on-chain, disrupting the hedger’s ability to manage their countertrade effectively and their hedging strategy.
The lack of control over the timing of the close means hedgers cannot ensure that their broker-side positions are closed appropriately, increasing the risk of slippage or other adverse trading conditions

Code Snippet

https://github.com/sherlock-audit/2024-06-symmetrical-update-2/blob/main/protocol-core/contracts/facets/PartyB/PartyBFacet.sol#L175

https://github.com/sherlock-audit/2024-06-symmetrical-update-2/blob/main/protocol-core/contracts/facets/PartyB/PartyBFacetImpl.sol#L267

Tool used

Manual Review

Recommendation

It is recommended to only allow PartyB to emergency close the positions, similar to the previous version of the protocol.

sherlock-audit / 2024-06-symmetrical-update-2-judging

xiaoming90 - PartyB unable to settle their hedges with their broker during emergency close #7