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

Execution Fee Refund Issue in `cancelFlow` Leading to Potential Revert

Summary

The cancelFlow function fully refunds the execution fee to the withdrawer, but since the gmxProxy has already paid the settlement transaction fee, this can lead to an issue where gmxProxy ends up covering the cost. If gmxProxy does not have enough ETH, the cancelFlow transaction reverts, preventing withdrawal cancellation.

Vulnerability Details

  • The withdraw function requires double the execution fee since it includes both the settlement and actual withdrawal transactions.

  • The settlement order is executed immediately, deducting the first execution fee.

  • If cancelFlow is called before the actual withdrawal transaction, it refunds the full execution fee to the withdrawer.

  • Since gmxProxy has already paid for the settlement transaction, it bears the cost.

  • If gmxProxy has insufficient ETH to cover the settlement fee, cancelFlow reverts, leading to a failed cancellation.

Impact

  • Users may be unable to cancel withdrawals if gmxProxy lacks funds.

  • Potential Loss of Funds from the gmxProxy.

POC

paste in /test/PerpetualVault.t.sol

function test_WithdrawExecutionFeeReturn() external {
address gmxProxy = address(PerpetualVault(vault).gmxProxy());
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
depositFixture(alice, 1e10);
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3380000000000000);
vm.prank(keeper);
PerpetualVault(vault).run(true, false, prices, data);
PerpetualVault.FLOW flow = PerpetualVault(vault).flow();
assertEq(uint8(flow), 2);
GmxOrderExecuted(true);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, new bytes[](0));
uint256[] memory depositIds = PerpetualVault(vault).getUserDeposits(
alice
);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(
false
);
uint256 lockTime = 1;
PerpetualVault(vault).setLockTime(lockTime);
vm.warp(block.timestamp + lockTime + 1);
payable(alice).transfer(1 ether);
vm.prank(alice);
uint bal = address(gmxProxy).balance;
PerpetualVault(vault).withdraw{value: executionFee * tx.gasprice}(
alice,
depositIds[0]
);
GmxOrderExecuted(true);
vm.prank(keeper);
PerpetualVault(vault).cancelFlow();
console.log((bal - address(gmxProxy).balance) > 0);
}

Recommendations

Partial Refund Instead of Full Refund: Modify cancelFlow to refund only the withdrawal execution fee while keeping the settlement execution fee covered.

Updates

Lead Judging Commences

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

finding_cancelFlow_can_be_called_between_settle_and_withdraw

Likelihood: None/Very Low, when any flow is cancelled after a settle and before a withdraw. No real reason to do that. Impact: High, loss of funds.

Support

FAQs

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

Give us feedback!