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

Wrong allocation of `executionFee` in `depositInfo`.

bizarro

Summary

User's order executionFee is stored in the depositInfo[depositId].executionFee, According to the protocol, users must pay an execution fee when a deposit or withdrawal triggers a GMX order execution; otherwise, the fee is refunded. The allocation of the execution fee is handled in the PerpetualVault::_payExecutionFee function, where, if msg.value exceeds minExecutionFee, the amount is transferred to the gmxProxy contract and stored as the execution fee. However, the execution fee calculation within _payExecutionFee is incorrect.

Vulnerability Details

  1. current state, positionIsClose == false, beenLong == false, leverage == 10_000

  2. User deposits 1e10 collateral token with msg.value as 2e18.

  3. The depositInfo[depositId].executionFee of user is stored as 1e18.

  4. Increase position order is placed on GMX.

  5. User is minted corresponding shares based on the depsoited value without getting any refund on the deposit.

  6. now, if the state changes to beenLong == true.

  7. User calls the withdraw function with msg.value as 1e10.

  8. User pays the executionFee which will be overridden to 1e18.

  9. And when the user receives the remaning of the executionFee, the amount received by the user will be less than what they deposited.

Proof of Concept(Coded)

Add this test to PerpetualVault.t.sol and run forge test --mt test_executionFeeReturn --via-ir --rpc-url arbitrum -vvv

function test_wrongTotalSharesCalculation() public {
address alice = makeAddr("alice");
payable(alice).transfer(1 ether);
depositFixture(alice, 1e10);
vm.deal(alice, 100 ether);
​
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](2);
data[0] = abi.encode(3380000000000000);
address keeper = PerpetualVault(vault).keeper();
vm.prank(keeper);
PerpetualVault(vault).run(true, false, prices, data);
GmxOrderExecuted(true);
bytes[] memory metadata = new bytes[](0);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, metadata);
​
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 amount = 1e12;
deal(address(collateralToken), alice, amount);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
vm.startPrank(alice);
collateralToken.approve(vault, amount);
​
// deposit collateral token when beenLong is false.
PerpetualVault(vault).deposit{value: (executionFee * tx.gasprice) + 2 ether}(amount);
vm.stopPrank();
uint256 userBalanceBeforeMinting = address(alice).balance;
​
uint8 flow = uint8(PerpetualVault(vault).flow());
assertEq(flow, 1);
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 1);
​
data[0] = abi.encode(3380000000000000);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, data);
​
GmxOrderExecuted(true);
// user's balance after minting shares
uint256 userBalanceAfterMinting = address(alice).balance;
​
// users balance before and after minting shares are equal.
assert(userBalanceBeforeMinting == userBalanceAfterMinting);
​
uint256[] memory depositIds = PerpetualVault(vault).getUserDeposits(alice);
assertEq(depositIds.length, 2);
console.log("ExecutionFee after deposit: ",PerpetualVault(vault).getUserExecutionFee(depositIds[1]));
(selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 6);
flow = uint8(PerpetualVault(vault).flow());
assertNotEq(flow, 0);
vm.prank(keeper);
delete data;
PerpetualVault(vault).runNextAction(prices, data);
(selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 0);
flow = uint8(PerpetualVault(vault).flow());
assertEq(flow, 0);
​
// withdraw collateral tokens when beenLong == true.
​
depositIds = PerpetualVault(vault).getUserDeposits(alice);
uint256 lockTime = PerpetualVault(vault).lockTime();
vm.warp(block.timestamp + lockTime + 1);
​
vm.prank(alice);
PerpetualVault(vault).withdraw{value: (executionFee * tx.gasprice) + 1 ether}(alice, depositIds[1]);
​
console.log("ExecutionFee after withdraw: ",PerpetualVault(vault).getUserExecutionFee(depositIds[1]));
}
Logs:
collateral/short/USDC amount: 10000000000
indexToken/ETH amount 0
ExecutionFee after deposit: 2000104667680000000
ExecutionFee after withdraw: 3000209335360000000

Impact

This could lead to wrong allocation of executionFee leading to user loosing funds while claiming the the executionFee as a refund.

Recommendations

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;
+ depositInfo[depositId].executionFee += msg.value;
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

invalid_no_refund

According the sponsor, here is the design choice: - when there's gmx interaction, no refund. - when there's no gmx interaction, refund fee. The remaining fees are used for gas cost of the keepers.

Support

FAQs

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