DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Valid

Over-Refunding Execution Fees

Summary

If PerpetualVault creates an Increase or Decrease order in GMX V2, the vault must pay an execution fee to GMX V2 to cover network costs. This fee is collected from users interacting with PerpetualVault.

However, in some cases, while the GMX V2 keeper is executing these orders, the order may fail for various reasons (e.g., slippage constraints, price impact limits). When this happens, GMX V2 partially refunds the execution fee to gmxProxy, as some of the fee is consumed by the keeper for executing the transaction.

Currently, if the PerpetualVault keeper calls cancelFlow(), the user receives a full refund of the execution fee. This is incorrect because part of the execution fee was already spent by the GMX V2 keeper. The user should only receive the remaining refunded amount, not the full execution fee, to ensure fairness and prevent unintended over-refunding.

Vulnerability Details

In GMX V2, an order can be canceled under two scenarios:

  1. Manual Cancellation – The order owner (PerpetualVault) cancels the order.

  2. Automatic Cancellation – The order fails to execute due to input parameters (e.g., acceptablePrice not being met) or other conditions.

PerpetualVault collects an execution fee from users during deposits and withdrawals to compensate GMX V2 keepers for executing orders. If an order is canceled, GMX V2 refunds the execution fee to gmxProxy.sol. However, the refund amount differs between the two scenarios, which can lead to an unfair refund to users.

Scenario 1: Manual Cancellation

When PerpetualVault cancels an order by calling cancelOrder(), GMX V2 fully refunds the execution fee to gmxProxy, since the GMX keeper never attempted execution.

Implementation in gmxProxy.sol

function cancelOrder() external {
require(msg.sender == perpVault, "invalid caller");
require(queue.requestKey != bytes32(0), "zero value");
gExchangeRouter.cancelOrder(queue.requestKey);
}

After the cancellation, if the keeper calls cancelFlow(), the user receives a full refund, which is fair since no gas was spent on execution.

function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(
depositInfo[depositId].owner,
depositInfo[depositId].amount
);
totalDepositAmount =
totalDepositAmount -
depositInfo[depositId].amount;
EnumerableSet.remove(
userDeposits[depositInfo[depositId].owner],
depositId
);
try
IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
)
{} catch {}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
try
IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
)
{} catch {}
}
// Setting flow to liquidation has no meaning.
// The aim is to run FINALIZE action (swap indexToken to collateralToken).
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}
function refundExecutionFee(address receipient, uint256 amount) external {
require(msg.sender == perpVault, "invalid caller");
payable(receipient).transfer(amount);
}

Since GMX V2 fully refunds the execution fee, the user receives the correct amount, making this scenario fair.

Scenario 2: Automatic Cancellation

If an order is automatically canceled due to execution conditions (e.g., acceptablePrice not being met), GMX V2 only partially refunds the execution fee. This is because the GMX keeper attempted to execute the transaction and burned some gas in the process. The remaining execution fee is then refunded to gmxProxy.

However, if the keeper calls cancelFlow() after the order is automatically canceled, the user still receives a full refund of the execution fee. This is problematic because GMX V2 has only refunded a partial amount to gmxProxy. As a result, the contract effectively over-refunds users.

POC :

To run add this test to PerpetualVault.t.sol and then

forge test --mt test_Run_Open2xLongPosition --fork-url <ARBITRUM RPC URL> -vv
function test_Run_Open2xLongPosition() external {
address keeper = PerpetualVault(vault2x).keeper();
address alice = makeAddr("alice");
depositFixtureInto2x(alice, 1e8);
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3390000000000000);
vm.prank(keeper);
PerpetualVault(vault2x).run(true, true, prices, data);
PerpetualVault.FLOW flow = PerpetualVault(vault2x).flow();
assertEq(uint8(flow), 2);
assertEq(PerpetualVault(vault2x).positionIsClosed(), true);
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault2x)
.nextAction();
assertEq(uint8(selector), 0);
GmxOrderExecuted2x(true);
vm.startPrank(keeper);
PerpetualVault(vault2x).runNextAction(prices, data);
vm.stopPrank();
bytes32 curPositionKey = PerpetualVault(vault2x).curPositionKey();
assertTrue(curPositionKey != bytes32(0));
assertEq(PerpetualVault(vault2x).beenLong(), true);
address BOB = makeAddr("bob");
vm.deal(BOB, 1 ether);
depositFixtureInto2x(BOB, 1e8);
uint initial = address(PerpetualVault(vault2x).gmxProxy()).balance;
console.log("INITIAL ETH BALANCE IN gmxProxy: ", initial);
data[0] = abi.encode(33); // Here setting acceptable Price too low so the order will be cancelled when GMX V2 keeper tries to execute it
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices, data);
uint colIn = IERC20(PerpetualVault(vault2x).collateralToken())
.balanceOf(vault2x);
console.log("COLIN: ", colIn);
uint medium = address(PerpetualVault(vault2x).gmxProxy()).balance;
console.log("MEDIUM ETH BALANCE IN gmxProxy: ", medium);
GmxOrderExecuted2x(false);
uint Final = address(PerpetualVault(vault2x).gmxProxy()).balance;
console.log("FINAL ETH BALANCE IN gmxProxy", Final);
uint col = IERC20(PerpetualVault(vault2x).collateralToken()).balanceOf(
vault2x
);
console.log("COL: ", col);
vm.prank(keeper);
PerpetualVault(vault2x).cancelFlow();
uint bobBal = address(BOB).balance;
assertEq(bobBal, 1 ether); // BOB getting full refund
}

Logs :

Ran 1 test for test/PerpetualVault.t.sol:PerpetualVaultTest
[PASS] test_Run_Open2xLongPosition() (gas: 6728903)
Logs:
INITIAL ETH BALANCE IN gmxProxy: 1000109394195081000
COLIN: 0
MEDIUM ETH BALANCE IN gmxProxy: 999881250052985000
FINAL ETH BALANCE IN gmxProxy 1000029725070231000
COL: 100000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 175.86ms (115.13ms CPU time)
Ran 1 test suite in 2.09s (175.86ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Users getting over refunded
Protocol loosing ETH funds

Tools Used

Manual review

Recommendations

To resolve this issue, the refund logic in cancelFlow() should account for the actual amount refunded by GMX V2. Instead of always refunding the full execution fee, the contract should distribute only the amount actually received from GMX V2.

Updates

Lead Judging Commences

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

finding_cancelFlow_can_be_called_after_order_execution_leading_to_disturb_shares_and_refund_too_many_fees

Likelihood: None/Very Low, when the keeper call cancelFlow after an order execution Impact: High, Inflation/deflation of total shares, and too many fees refunded.

Support

FAQs

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

Give us feedback!