DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

PerpetualVault Lacks Functions to Withdraw Execution Fees, Leading to Stuck Funds

Summary

Because the PerpetualVault is set as both the order receiver and cancellationReceiver but lacks a way to accept or transfer ETH/WETH, any refunded execution fees remain trapped in the vault.

Vulnerability Details

When GMXProxy builds an order before sending to GMX, both the receiver and cancellationReceiver is set to the PerpetualVault:

function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
...SNIP...
CreateOrderParamsAddresses memory paramsAddresses = CreateOrderParamsAddresses({
@> receiver: perpVault,
@> cancellationReceiver: address(perpVault),
callbackContract: address(this),
uiFeeReceiver: address(0),
market: orderData.market,
initialCollateralToken: orderData.initialCollateralToken,
swapPath: orderData.swapPath
});
...SNIP...
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey;
return requestKey;
}

If an order is canceled, the GMXProxy calls ExchangeRouter::cancelOrder(). When GMX is canceling the order, OrderUtils.sol::cancelOrder() calls GasUtils.payexecutionFee passing in both the callback contract and the cancel receiver addresses:

function cancelOrder(CancelOrderParams memory params) public {
...SNIP...
@> address executionFeeReceiver = order.cancellationReceiver();
if (executionFeeReceiver == address(0)) {
executionFeeReceiver = order.receiver();
}
EventUtils.EventLogData memory eventData;
CallbackUtils.afterOrderCancellation(params.key, order, eventData);
GasUtils.payExecutionFee(
params.dataStore,
params.eventEmitter,
params.orderVault,
params.key,
@> order.callbackContract(),
order.executionFee(),
params.startingGas,
GasUtils.estimateOrderOraclePriceCount(order.swapPath().length),
params.keeper,
@> executionFeeReceiver
);
}

payExecutionFee() tries to refund the execution fee to the callback contract first, but if this fails (e.g. out of gas), it will then try to send the refund to the cancellation receiver in the form of ETH (or wETH if ETH fails).

function payExecutionFee(
DataStore dataStore,
EventEmitter eventEmitter,
StrictBank bank,
bytes32 key,
address callbackContract,
uint256 executionFee,
uint256 startingGas,
uint256 oraclePriceCount,
address keeper,
address refundReceiver
) external {
...SNIP...
@> cache.refundWasSent = CallbackUtils.refundExecutionFee(dataStore, key, callbackContract, cache.refundFeeAmount, eventData);
if (cache.refundWasSent) {
emitExecutionFeeRefundCallback(eventEmitter, callbackContract, cache.refundFeeAmount);
} else {
@> TokenUtils.sendNativeToken(dataStore, refundReceiver, cache.refundFeeAmount);
emitExecutionFeeRefund(eventEmitter, refundReceiver, cache.refundFeeAmount);
}
}

Since the cancellation receiver is the PerpetualVault, but the Perpetual Vault can't receive ETH or wETH, GMX will end up transferring wETH to the PerpetualVault address, but those funds will be stuck because there's no way to transfer them out.

Impact

Stuck funds

Tools Used

Manual review

Recommendations

Add a receive function and a way to handle the transfer of ETH and wETH from the perpetuals vault.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_cancellationReceiver_set_to_perpVault_will_stuck_executionFee_in_vault

Likelihood: Low/Medium, during cancellation with a refund. Impact: High, refund and ETH are stuck in the perpVault.

Appeal created

sakshamseth5 Auditor
9 months ago
n0kto Lead Judge
8 months ago
n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!