Summary
GmxProxy.sol#settle() function missed the check that execution feature of decrease order is enabled.
Vulnerability Details
GmxProxy.sol#createOrder() function is as follows.
function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
uint256 positionExecutionFee = getExecutionGasLimit(
orderType,
orderData.callbackGasLimit
) * tx.gasprice;
require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);
@> bytes32 executeOrderFeatureKey = keccak256(
abi.encode(
EXECUTE_ORDER_FEATURE_DISABLED,
orderHandler,
orderType
)
);
require(
@> dataStore.getBool(executeOrderFeatureKey) == false,
"gmx execution disabled"
);
...
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey;
return requestKey;
}
As we can see above, createOrder() function checked that execution feature is enabled.
But settle() function missed it.
function settle(
IGmxProxy.OrderData memory orderData
) external returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
uint256 positionExecutionFee = getExecutionGasLimit(
Order.OrderType.MarketDecrease,
orderData.callbackGasLimit
) * tx.gasprice;
require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);
gExchangeRouter.sendWnt{value: positionExecutionFee}(
orderVault,
positionExecutionFee
);
...
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey;
queue.isSettle = true;
return requestKey;
}
So if execution feature is disabled for MarketDecrease, the order is canceled.
Then, PerpetualVault.sol#afterOrderCancellation() function triggeres settle, again.
function afterOrderCancellation(
bytes32 requestKey,
Order.OrderType orderType,
IGmxProxy.OrderResultData memory orderResultData
) external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
if (orderResultData.isSettle) {
@> nextAction.selector = NextActionSelector.SETTLE_ACTION;
} else if (orderType == Order.OrderType.MarketSwap) {
...
} else {
...
}
emit GmxPositionCallbackCalled(requestKey, false);
}
So keeper have to cancel flow to recover protocol's state.
Therefore, whenever a user withdraws in case of positionIsClosed = false, keeper has to cancel flow.
Impact
Keeper suffers from continued canceling flow.
So a malicious user can freeze protocol with continued fake withdrawing.
Tools Used
Manual Review
Recommendations
Modify GmxProxy.sol#settle() function as follows.
function settle(
IGmxProxy.OrderData memory orderData
) external returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
++
++ bytes32 executeOrderFeatureKey = keccak256(
++ abi.encode(
++ EXECUTE_ORDER_FEATURE_DISABLED,
++ orderHandler,
++ Order.OrderType.MarketDecrease
++ )
++ );
++ require(
++ dataStore.getBool(executeOrderFeatureKey) == false,
++ "gmx execution disabled"
++ );
...
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey;
queue.isSettle = true;
return requestKey;
}
