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

Silently failing execution fee refunds in PerpetualVault can lead to permanently locked fund


Summary and Impact

The PerpetualVault contract implements a fee refund mechanism that silently fails when users can't receive ETH refunds, leading to permanently locked execution fees in the protocol. This creates an economic imbalance where users who are unable to receive ETH transfers (like certain smart contracts) lose their excess execution fees with no recourse to recover them.

While reviewing the protocol's documentation and invariants, particularly the "Fair Share" principle that states "Depositors should not be able to claim more than their fair share", I noticed this issue violates the economic fairness of the system. Some users effectively pay higher fees than others for the same operations, simply because their excess fees get stuck.

The protocol documentation emphasizes the importance of fair fee distribution, yet this implementation can result in uneven fee treatment among users.


Vulnerability Details

The issue occurs in two critical functions: _mint() and _handleReturn(). Both attempt to refund excess execution fees using a try-catch pattern that silently fails:

// In _mint():
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[counter].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}
// In _handleReturn():
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}

The key issues are:

  1. Failed refunds are silently caught with empty catch blocks

  2. No event emission on refund failure

  3. No tracking of failed refunds

  4. No mechanism to recover stuck fees

Here's a test demonstrating the issue:

function test_RefundFailure() public {
// Setup a contract that can't receive ETH
NoETHReceiver user = new NoETHReceiver();
deal(address(user), 100 ether);
// Track initial balances
uint256 initialBalance = address(gmxProxy).balance;
// User deposits with excess execution fee
uint256 executionFee = vault.getExecutionGasLimit(true) * tx.gasprice;
user.deposit{value: executionFee * 2}(vault, minDepositAmount);
// Complete deposit flow
keeper.completeDepositFlow();
// Verify excess fee is stuck
assertEq(address(gmxProxy).balance, initialBalance + executionFee * 2);
// User should have received executionFee back but didn't
}

This particularly impacts:

  1. Smart contract wallets with complex receive functions

  2. Contracts without payable fallbacks

  3. Users who lose access to their original depositing address

Looking at the project invariants, specifically "Fair Share of Funding Fees", this behavior creates an unfair situation where some users permanently lose their excess fees while others don't.


Tools Used

  • Manual Review

  • Foundry


Recommendations

Implement a pull pattern for fee refunds:

  1. Add a mapping to track refundable fees:

mapping(address => uint256) public pendingRefunds;
  1. Instead of direct transfers, accumulate refunds:

pendingRefunds[user] += refundAmount;
  1. Add a claim function:

function claimRefund() external {
uint256 refundAmount = pendingRefunds[msg.sender];
require(refundAmount > 0, "No refund available");
pendingRefunds[msg.sender] = 0;
(bool success,) = msg.sender.call{value: refundAmount}("");
require(success, "Refund failed");
}

This ensures users can always recover their excess fees and maintains the protocol's commitment to fair fee distribution.

Updates

Lead Judging Commences

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

Support

FAQs

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