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

ETH Lock Vulnerability in PerpetualVault Deposit Function

karl

Summary

The deposit function in PerpetualVault contract is marked as payable and accepts ETH, but lacks proper ETH handling logic. This allows users to send excess ETH beyond the required execution fee, which becomes permanently locked in the contract , while there is no rescue function for it .

Vulnerability Details

The deposit function accepts ETH payments for execution fees but does not validate whether the sent ETH amount matches the required execution fee especially when position is closed . Any excess ETH sent with the transaction becomes trapped in the contract with no withdrawal mechanism.

Impact

Users who accidentally send more ETH than the required execution fee will permanently lose access to their excess ETH, as there is no mechanism to recover these funds.

Proof Of Concept(coded)

Here is the relevant fuzz test in foundry that could be added to file PerpetualVault.t.sol :

function testFuzz_Deposit_ExcessETH(uint96 amount) external {
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
address alice = makeAddr("alice");
​
//Set the amount to be a random number between the min and max deposit amount
uint256 maxDeposit = PerpetualVault(vault).maxDepositAmount();
uint256 minDeposit = PerpetualVault(vault).minDepositAmount();
amount = uint96(bound(amount, minDeposit, maxDeposit));
vm.startPrank(alice);
deal(address(collateralToken), alice, amount);
deal(alice, 2 ether);
//Get the execution fee
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
//Set the excess amount to be the execution fee plus 1 ether
//While in original test we only set the ideal case , i.e.
//uint256 excessAmount = executionFee * tx.gasprice
uint256 excessAmount = executionFee * tx.gasprice + 1 ether;
//Log Initial Balance
uint256 vaultBalanceBefore = address(vault).balance;
collateralToken.approve(vault, amount);
//Execute deposit
PerpetualVault(vault).deposit{value: excessAmount}(amount);
//Get vault balance after we deposit
uint256 vaultBalanceAfter = address(vault).balance;
//Check if the excess amount is locked in the contract
assertEq(
vaultBalanceAfter - vaultBalanceBefore,
excessAmount,
"Excess ETH should be locked in contract"
);
//Check if the deposit is successful
uint256[] memory depositIds = PerpetualVault(vault).getUserDeposits(alice);
assertEq(depositIds.length, 1);
vm.stopPrank();
}

And here is the log output from it :
[PASS] testFuzz_Deposit_ExcessETH(uint96) (runs: 258, μ: 486078, ~: 486220)

Tools Used

Manual

Recommendations

Add msg.value related validation to condition branch of function Deposit when positionIsClosed is true , or implement function withdrawEth just like in contract GmxProxy .

Updates

Lead Judging Commences

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

Users mistake, only impacting themselves.

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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