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

Steal execution fees meant to be refunded to other users

sauronsol

Summary

A critical vulnerability in the _handleReturn function of PerpetualVault.sol allows an attacker to steal execution fees meant to be refunded to other users by exploiting an incorrect variable reference. The same vulnerability is also present in the _mint and _cancelFlow functions, creating a systemic issue throughout the contract.

Severity: High

The vulnerability results in direct theft of funds (execution fees) with minimal effort and can be repeatedly exploited against multiple victims.

Vulnerability Details

The vulnerability is in the _handleReturn function of PerpetualVault.sol, specifically in the code that refunds unused execution fees to users. The function incorrectly refunds fees to depositInfo[counter].owner instead of depositInfo[depositId].owner:

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 issue is that counter is a global variable representing the last deposit ID, while depositId is the ID of the specific deposit being withdrawn. This means that execution fee refunds are always sent to the most recent depositor rather than the user who initiated the withdrawal.

The same issue exists in other critical functions:

  1. In the _mint function (around line 787), where execution fees are refunded incorrectly:

if (depositInfo[depositId].executionFee > 0) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee) {} catch {}
}

  1. In the _cancelFlow function (around lines 1227-1228 and 1237-1238), similar incorrect refunds occur:

if (depositInfo[depositId].executionFee > 0) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee) {} catch {}
}

Below is a Proof of Concept demonstrating this vulnerability in the _handleReturn function:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import {Test, console} from "forge-std/Test.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
/**
* @title ExecutionFeeExploitTest
* @notice Test case demonstrating a vulnerability in PerpetualVault's _handleReturn function
* where execution fees are refunded to the wrong user (counter instead of depositId)
*/
contract ExecutionFeeExploitTest is Test {
// Mock contract to simulate PerpetualVault behavior
MockPerpetualVault vault;
// Mock GmxProxy to track refunds
MockGmxProxy gmxProxy;
// Test accounts
address public alice = address(0x1);
address public bob = address(0x2);
address public admin = address(0x3);
// Execution fee (0.01 ETH)
uint256 public constant EXECUTION_FEE = 0.01 ether;
uint256 public constant USED_FEE = 0.002 ether;
uint256 public constant REFUND_AMOUNT = EXECUTION_FEE - USED_FEE;
function setUp() public {
// Give test accounts some ETH
vm.deal(alice, 10 ether);
vm.deal(bob, 10 ether);
vm.deal(admin, 10 ether);
// Deploy mock contracts
gmxProxy = new MockGmxProxy();
vault = new MockPerpetualVault(address(gmxProxy));
// Setup initial state
vm.startPrank(admin);
// Setup Alice's deposit (ID 1)
vault.setDepositInfo(1, alice, 5000e6, 5000e6, EXECUTION_FEE);
// Setup Bob's deposit (ID 2)
vault.setDepositInfo(2, bob, 1000e6, 1000e6, 0.001 ether);
// Set counter to Bob's deposit ID
vault.setCounter(2);
// Set callback gas limit
vault.setCallbackGasLimit(200000);
vm.stopPrank();
}
function testExecutionFeeExploit() public {
console.log("Starting execution fee vulnerability test");
console.log("Initial state:");
console.log(" Alice's deposit ID: 1");
console.log(" Alice's execution fee:", EXECUTION_FEE);
console.log(" Bob's deposit ID: 2");
console.log(" Current counter (last deposit): 2");
// Set transaction gas price
vm.txGasPrice(10 gwei);
// Simulate Alice withdrawing (flowData = Alice's deposit ID)
vm.prank(admin);
vault.simulateHandleReturn(1, true);
// Check refund recipient and amount
address refundedTo = gmxProxy.lastRefundedTo();
uint256 refundedAmount = gmxProxy.lastRefundedAmount();
console.log("\nRefund results:");
console.log(" Refund was sent to:", refundedTo);
console.log(" Refund amount:", refundedAmount);
// Assert that the refund went to Bob (counter's owner) instead of Alice (depositId's owner)
assertEq(refundedTo, bob, "Refund was sent to the wrong address");
assertEq(refundedAmount, REFUND_AMOUNT, "Refund amount is incorrect");
console.log("\nVULNERABILITY CONFIRMED: Execution fee was refunded to Bob instead of Alice");
console.log("The bug is in _handleReturn() where it uses depositInfo[counter].owner instead of depositInfo[depositId].owner");
}
}
​
// Simple mock for GmxProxy to track refunds
contract MockGmxProxy {
address public lastRefundedTo;
uint256 public lastRefundedAmount;
function refundExecutionFee(address to, uint256 amount) external {
lastRefundedTo = to;
lastRefundedAmount = amount;
}
}
​
// Mock for PerpetualVault that simulates the vulnerability
contract MockPerpetualVault {
address public gmxProxy;
uint256 public counter;
uint256 public callbackGasLimit;
struct DepositInfo {
address owner;
uint256 shares;
uint256 amount;
uint256 executionFee;
}
mapping(uint256 => DepositInfo) public depositInfo;
constructor(address _gmxProxy) {
gmxProxy = _gmxProxy;
}
function setDepositInfo(
uint256 id,
address owner,
uint256 shares,
uint256 amount,
uint256 executionFee
) external {
depositInfo[id].owner = owner;
depositInfo[id].shares = shares;
depositInfo[id].amount = amount;
depositInfo[id].executionFee = executionFee;
}
function setCounter(uint256 _counter) external {
counter = _counter;
}
function setCallbackGasLimit(uint256 _callbackGasLimit) external {
callbackGasLimit = _callbackGasLimit;
}
// This function simulates the vulnerable part of _handleReturn
function simulateHandleReturn(uint256 depositId, bool refundFee) external {
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
// THIS IS THE BUG: Using counter instead of depositId
MockGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, // BUG: Should be depositInfo[depositId].owner
depositInfo[depositId].executionFee - usedFee
);
}
}
}
}

The PoC demonstrates that when Alice (who deposited first) withdraws her funds, the refund of unused execution fees is sent to Bob (the latest depositor) instead of Alice.

Impact

This vulnerability has a severe financial impact:

  • Direct theft of users' execution fees (which can be substantial in high gas environments)

  • The vulnerability is easily and repeatedly exploitable by attackers who monitor the mempool for withdraw transactions

  • Any user who makes a deposit becomes the recipient of all subsequent execution fee refunds until another deposit is made

  • Users who paid high execution fees for priority processing lose a significant portion of those fees to unrelated parties

Numerical proof: In our PoC, Alice loses 0.008 ETH (worth potentially tens or hundreds of dollars depending on ETH price) to Bob, who simply made a deposit after her.

Tools Used

  • Foundry for writing and executing the proof of concept test

  • Manual code analysis of PerpetualVault.sol

Recommendations

To fix this vulnerability, the code should be corrected to refund execution fees to the appropriate owner:

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

This is a straightforward fix that ensures execution fee refunds are sent to the correct user who initiated the withdrawal.

Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_counter_invalid_during_cancelFlow_after_withdrawing

Likelihood: Low, contract has to call cancelFlow after a withdraw, and the settle action is already executed by GMX. Impact: High, the fees will be distributed to the last depositor and not the withdrawer.

finding_counter_invalid_during_handleReturn

Likelihood: Medium/High, when withdraw on a 1x vault. Impact: High, the fees will be distributed to the last depositor and not the withdrawer.

Appeal created

0xaman Auditor
8 months ago
n0kto Lead Judge
8 months ago
n0kto Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_counter_invalid_during_cancelFlow_after_withdrawing

Likelihood: Low, contract has to call cancelFlow after a withdraw, and the settle action is already executed by GMX. Impact: High, the fees will be distributed to the last depositor and not the withdrawer.

finding_counter_invalid_during_handleReturn

Likelihood: Medium/High, when withdraw on a 1x vault. Impact: High, the fees will be distributed to the last depositor and not the withdrawer.

Support

FAQs

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