You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Jan 7, 2024. It is now read-only.
sherlock-admin opened this issue
Jul 3, 2023
· 0 comments
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHighA valid High severity issueRewardA payout will be made for this issue
Emergency position closing can be griefed by Party A
Summary
Party B's emergency position closing attempt via the emergencyClosePosition function can be front-run by Party A by tactically incrementing Party A's nonce, leading to the invalidation of the signature used by Party B and causing the emergency close to reverting.
Vulnerability Detail
Party B can emergency close (i.e., completely filling) an opened position via the emergencyClosePosition function in the PartyBFacetImpl library, carrying out this action at the current market price denoted by upnlSig.price. The provided struct parameter upnlSig is verified via the LibMuon.verifyPairUpnlAndPrice function to ensure the struct is valid and legitimate and the price is not manipulated/stale.
Internally, the verifyPairUpnlAndPrice function calculates the hash with one of the values being the current nonce of Party A (AccountStorage.layout().partyANonces[partyA]).
However, Party A can increase the nonce in the meantime, for example, by using the AccountFacet.allocate function to allocate a small amount of deposited funds. This results in an increased nonce of Party A. If Party A front-runs the emergency close by Party B and purposefully increases its nonce, the signature used by Party B is invalidated, causing the signature verification to fail and, thus, the emergency close to revert.
The only way to prevent this from happening during an emergency close is to pause the accounting globally for everyone so that the whenNotAccountingPaused modifier reverts and allocating (i.e., increasing Party A's nonce) is not possible for the duration of the emergency close. Please note that this emergency mode can be enabled either globally or for specific Party B addresses. This aforementioned issue is particularly significant in the latter scenario, as a global pause would affect all other Party A participants in the protocol.
Impact
Party A can prevent Party B from closing the position via the emergencyClosePosition function and thus keep the position open for longer to potentially profit from it if the price moves in Party A's favor (or prevent emergency closing due to other reasons).
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
DuplicateA valid issue that is a duplicate of an issue with `Has Duplicates` labelHighA valid High severity issueRewardA payout will be made for this issue
berndartmueller
medium
Emergency position closing can be griefed by Party A
Summary
Party B's emergency position closing attempt via the
emergencyClosePosition
function can be front-run by Party A by tactically incrementing Party A's nonce, leading to the invalidation of the signature used by Party B and causing the emergency close to reverting.Vulnerability Detail
Party B can emergency close (i.e., completely filling) an opened position via the
emergencyClosePosition
function in thePartyBFacetImpl
library, carrying out this action at the current market price denoted byupnlSig.price
. The provided struct parameterupnlSig
is verified via theLibMuon.verifyPairUpnlAndPrice
function to ensure the struct is valid and legitimate and the price is not manipulated/stale.Internally, the
verifyPairUpnlAndPrice
function calculates the hash with one of the values being the current nonce of Party A (AccountStorage.layout().partyANonces[partyA]
).However, Party A can increase the nonce in the meantime, for example, by using the
AccountFacet.allocate
function to allocate a small amount of deposited funds. This results in an increased nonce of Party A. If Party A front-runs the emergency close by Party B and purposefully increases its nonce, the signature used by Party B is invalidated, causing the signature verification to fail and, thus, the emergency close to revert.The only way to prevent this from happening during an emergency close is to pause the accounting globally for everyone so that the
whenNotAccountingPaused
modifier reverts and allocating (i.e., increasing Party A's nonce) is not possible for the duration of the emergency close. Please note that this emergency mode can be enabled either globally or for specific Party B addresses. This aforementioned issue is particularly significant in the latter scenario, as a global pause would affect all other Party A participants in the protocol.Impact
Party A can prevent Party B from closing the position via the
emergencyClosePosition
function and thus keep the position open for longer to potentially profit from it if the price moves in Party A's favor (or prevent emergency closing due to other reasons).Code Snippet
contracts/facets/PartyB/PartyBFacetImpl.sol#L313
contracts/libraries/LibMuon.sol#L180
Tool used
Manual Review
Recommendation
Consider limiting the usage of Party A's actions, which can increase the nonce, during the time the emergency mode is active.
Duplicate of #233
The text was updated successfully, but these errors were encountered: