Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Cancelled orders can DoS protocol

ccvascocc

Summary

Function afterOrderCancellation reinstates the nextAction data of a failed action in an attempt to retry its execution. However, there is no guarantee that the underlying reason for the error isn't still in place.

Vulnerability Details

The vulnerability lies in function afterOrderCancellation, which is triggered whenever an order is cancelled in GMX. This function will set the stage for the same action to be reprocessed, by setting the nextAction.selector. This is done regardless of the conditions or the reasons for the unexpected cancellation.

function afterOrderCancellation(
bytes32 requestKey,
Order.OrderType orderType,
IGmxProxy.OrderResultData memory orderResultData
) external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
//@audit
if (orderResultData.isSettle) {
// Retry settle action.
nextAction.selector = NextActionSelector.SETTLE_ACTION;
} else if (orderType == Order.OrderType.MarketSwap) {
// If GMX swap fails, retry in the next action.
nextAction.selector = NextActionSelector.SWAP_ACTION;
// abi.encode(swapAmount, swapDirection): if swap direction is true, swap collateralToken to indexToken
nextAction.data = abi.encode(swapProgressData.remaining, swapProgressData.isCollateralToIndex);
} else {
if (flow == FLOW.DEPOSIT) {
nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(beenLong);
} else if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
} else {
// If signal change fails, the offchain script starts again from the current status.
delete flowData;
delete flow;
}
}
emit GmxPositionCallbackCalled(requestKey, false);
}

Impact

Since this protocol only allows one action to be executed at a time, the protocol can get stuck for long periods if the reason for this error is not resolved.

Tools Used

Manual Review

Recommendations

Consider killing the flow and require a new one to be initiated by the user.

Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.