Summary
The PerpetualVault contract contains a critical issue where execution fees can become permanently trapped due to the unlimited payment acceptance and withdrawal mechanism in GmxProxy being inadequate which results in overpaid execution fees being unrecoverable, even by the contract owner.
Vulnerability Details
function _payExecutionFee(uint256 depositId, bool isDeposit) internal {
uint256 minExecutionFee = getExecutionGasLimit(isDeposit) * tx.gasprice;
if (msg.value < minExecutionFee) {
revert Error.InsufficientAmount();
}
if (msg.value > 0) {
payable(address(gmxProxy)).transfer(msg.value);
depositInfo[depositId].executionFee = msg.value;
}
}
The _payExecutionFee function stores the entire msg.value as an execution fee which should only store the minimum amount required (minExecutionFee). Any excess ETH sent by the user is not returned.
function withdrawEth() external onlyOwner returns (uint256) {
uint256 balance = address(this).balance;
payable(msg.sender).transfer(balance);
return balance;
}
The withdrawEth function only handles ETH that was accidentally sent but does not handle where the excess ETH is for fees.
Users who send more ETH than necessary cannot get the excess back, the funds are stuck in the GMX proxy contract.
Impact
Excess fees will be trapped and the contract owner will not be able to withdraw them.
POC
Add this to PerpetualVault.t.sol and run it forge test --match-test test_ExecutionFeeOverpayment --rpc-url arbitrum -vvvv.
function test_ExecutionFeeOverpayment() external {
address alice = makeAddr("alice");
uint256 initialBalance = 2 ether;
deal(alice, initialBalance);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true) * tx.gasprice;
uint256 depositAmount = 1e10;
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
vm.startPrank(alice);
deal(address(collateralToken), alice, depositAmount);
collateralToken.approve(vault, depositAmount);
uint256 overpayment = executionFee * 2;
uint256 aliceBalanceBefore = alice.balance;
PerpetualVault(vault).deposit{value: overpayment}(depositAmount);
vm.stopPrank();
uint256[] memory depositIds = PerpetualVault(vault).getUserDeposits(alice);
(, , , uint256 storedFee, , ) = PerpetualVault(vault).depositInfo(depositIds[0]);
assertEq(storedFee, overpayment, "Full overpayment was stored as execution fee");
assertEq(
aliceBalanceBefore - alice.balance,
overpayment,
"User lost full overpayment amount"
);
address payable gmxProxyAddr = payable(address(PerpetualVault(vault).gmxProxy()));
uint256 trappedEth = gmxProxyAddr.balance;
assertTrue(trappedEth > 0, "ETH should be trapped in GmxProxy");
address owner = GmxProxy(gmxProxyAddr).owner();
uint256 ownerBalanceBefore = owner.balance;
vm.prank(owner);
vm.deal(gmxProxyAddr, trappedEth);
bool success;
bytes memory result;
(success, result) = address(gmxProxyAddr).call(
abi.encodeWithSignature("withdrawEth()")
);
assertEq(gmxProxyAddr.balance, trappedEth, "ETH should remain trapped after withdrawal attempt");
assertEq(
owner.balance,
ownerBalanceBefore,
"Owner balance should not change"
);
vm.expectRevert();
(success, ) = address(gmxProxyAddr).call{value: 0}("");
}
Result:
├─ [7986] TransparentUpgradeableProxy::fallback()
│ ├─ [7363] GmxProxy::withdrawEth() [delegatecall]
│ │ ├─ [43] PerpetualVaultTest::fallback{value: 1000000000000000000}()
│ │ │ └─ ← [Revert] EvmError: Revert
│ │ └─ ← [Revert] EvmError: Revert
│ └─ ← [Revert] EvmError: Revert
ETH is trapped in GmxProxy and cannot be retrieved.
├─ [0] VM::assertEq(1000000000000000000 [1e18], 1000000000000000000 [1e18], "ETH should remain trapped after withdrawal attempt")
Even the owner cannot withdraw the trapped ETH.
├─ [0] VM::expectRevert(custom error 0xf4844814)
├─ [684] TransparentUpgradeableProxy::receive()
│ ├─ [55] GmxProxy::receive() [delegatecall]
│ │ └─ ← [Stop]
Attempts to access trapped ETH always fail.
Tools Used
Recommendations
Implement strict execution fee limits in PerpetualVault.
function _payExecutionFee(uint256 depositId, bool isDeposit) internal {
uint256 requiredFee = getExecutionGasLimit(isDeposit) * tx.gasprice;
if (msg.value < requiredFee) {
revert Error.InsufficientAmount();
}
if (msg.value > requiredFee) {
revert Error.ExcessiveExecutionFee();
}
payable(address(gmxProxy)).transfer(requiredFee);
depositInfo[depositId].executionFee = requiredFee;
}
