Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

The `PerpetualVault::deposit` function does not properly handle execution fee refunds for immediate deposits, leading to stuck or loss of funds.

summer

Summary

The documentation mentions that execution fees are for GMX operations. But when the position is closed, the code doesn't refund the excess fee. In GMX, creating orders requires paying gas, and the execution fee is meant to cover that. However, if the vault immediately processes the deposit without needing a GMX order (because the position is closed), the full fee isn't used. But the code doesn't refund the difference

Vulnerability Details

The Issue arises in the PerpetualVault::deposit function through a fundamental design error where the refundFee parameter is set to false when positionIsClosed is true, despite the protocol's intention to handle immediate deposits differently.

//PerpetualVault::deposit
​
if (positionIsClosed) {
MarketPrices memory prices;
_mint(counter, amount, false, prices); // refundFee is false!
_finalize(hex'');
}
​

This leads to incorrect fee behavior, preventing expected refunds. The fee collection mechanism calculates minimum execution fees based on gas limits, verifies that users provide sufficient funds, stores the exact fee amount in depositInfo, and transfers the fees to the GMX Proxy. However, while the _mint includes refund logic, it is never executed, resulting in excess fees being retained. The protocol maintains detailed tracking of execution fees, and the depositInfo structure stores these amounts, but the absence of a refund process contradicts the intended design.

Impact

The impact of this vulnerability is significant, it indicates a disconnect between design and implementation cos users lose excess ETH sent as execution fees, leading to unnecessary fee retention by the protocol

POC

Place the following test in test/PerpetualVault.t.sol run with -vvv;
This test is meant to fail if the vault were to refund the extra fee—but given the design (with refundFee set to false for closed positions), it should pass by demonstrating that the entire fee is deducted and never refunded.

function test_ExecutionFeeRefundIssue() external {
// Use a test account for the deposit.
address alice = makeAddr("alice");
vm.startPrank(alice);
​
// Fund alice with enough ETH so she can cover the deposit fees.
deal(alice, 1 ether);
​
// Prepare deposit parameters.
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 depositAmount = PerpetualVault(vault).minDepositAmount();
// Fund alice with the necessary ERC20 collateral.
deal(address(collateralToken), alice, depositAmount);
collateralToken.approve(vault, depositAmount);
​
// Calculate the base execution fee from the vault.
uint256 baseExecutionFee = PerpetualVault(vault).getExecutionGasLimit(true) * tx.gasprice;
// Add an extra fee that ideally should be refunded.
uint256 extraFee = 0.01 ether;
uint256 totalFee = baseExecutionFee + extraFee;
​
// Log key details.
console.log("Alice ETH balance before deposit:", alice.balance);
console.log("Base execution fee:", baseExecutionFee);
console.log("Extra fee:", extraFee);
console.log("Total fee to be sent:", totalFee);
​
uint256 balanceBefore = alice.balance;
​
// Make the deposit. Note: when the position is closed, _mint is called with refundFee = false.
PerpetualVault(vault).deposit{value: totalFee}(depositAmount);
​
uint256 balanceAfter = alice.balance;
console.log("Alice ETH balance after deposit:", balanceAfter);
console.log("ETH deducted from alice:", balanceBefore - balanceAfter);
​
// Since refund logic is bypassed, the deducted ETH should be at least totalFee (plus any gas cost).
uint256 deducted = balanceBefore - balanceAfter;
assertGe(deducted, totalFee);
​
vm.stopPrank();
}
​
​

Tools Used

Manual review

Recommendations

change refundfee to true

if (positionIsClosed) {
MarketPrices memory prices;
- _mint(counter, amount, false, prices); // refundFee is false
+ _mint(counter, amount, true, prices); // Change refundFee to true
_finalize(hex'');
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_deposit_positionIsClosed_does_not_refund_fees

Fees are not collected during that scenario, so no refund needed.

Support

FAQs

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