DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Users could create orders with minimal execution fees

Summary

When a user creates an order through GmxProxy with an execution fee (msg.value) that's below the contract's minEth threshold (0.002 ETH). createOrder() should revert in this case, but the implementation allows it through.

require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);

This checks the contract's total balance rather than the specific msg.value for the order. Why? Because the contract may have accumulated ETH from previous operations.

Users could create orders with minimal execution fees, relying on the contract's existing ETH balance. This could:

  1. Lead to order execution failures if the contract's ETH balance drops

  2. Unfairly subsidize some users' execution fees using the contract's ETH reserves

Vulnerability Details

The Perpetual Vault Protocol uses a keeper system to execute GMX trades, with each trade requiring an execution fee to compensate keepers. The protocol sets a minimum fee threshold (minEth = 0.002 ETH) to ensure keepers are properly incentivized. However, there's an issue in how these fees are validated.

When a user initiates a trade through the GmxProxy contract, they're supposed to provide enough ETH to cover keeper costs. The contract attempts to enforce this through a balance check, but here's where things get interesting, instead of validating the specific transaction's fee (msg.value), it looks at the contract's total ETH balance. Iit should be validating msg.value against minEth instead of checking the contract's total balance.

function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
// 🔢 Calculate required execution fee
uint256 positionExecutionFee = getExecutionGasLimit(
orderType,
orderData.callbackGasLimit
) * tx.gasprice;
// 🚨 VULNERABILITY: Checks total contract balance instead of msg.value
require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);
// 🔍 Feature check
bytes32 executeOrderFeatureKey = keccak256(
abi.encode(
EXECUTE_ORDER_FEATURE_DISABLED,
orderHandler,
orderType
)
);
require(
dataStore.getBool(executeOrderFeatureKey) == false,
"gmx execution disabled"
);
// 💸 Transfer execution fee to GMX
gExchangeRouter.sendWnt{value: positionExecutionFee}(
orderVault,
positionExecutionFee
);
// 🔄 Handle token transfers for swaps and increases
if (
orderType == Order.OrderType.MarketSwap ||
orderType == Order.OrderType.MarketIncrease
) {
IERC20(orderData.initialCollateralToken).safeApprove(
address(gmxRouter),
orderData.amountIn
);
gExchangeRouter.sendTokens(
orderData.initialCollateralToken,
orderVault,
orderData.amountIn
);
}
// 📝 Set up order parameters
CreateOrderParamsAddresses memory paramsAddresses = CreateOrderParamsAddresses({...});
CreateOrderParamsNumbers memory paramsNumber = CreateOrderParamsNumbers({...});
// ✍️ Create and queue the order
CreateOrderParams memory params = CreateOrderParams({...});
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey;
return requestKey;
}

The contract checks if it has enough total ETH to pay keepers, but doesn't verify that the user actually contributed their fair share. This means a user could send 0.001 ETH (below minEth) and still create an order if the contract happens to have leftover ETH from previous operations.

Think of this like a shared taxi service where passengers are supposed to pay their own fare, but the system only checks if there's enough money in the collective pool. Early riders might overpay, inadvertently subsidizing later passengers who underpay.

In the protocol's context, this could lead to:

  • Keepers receiving insufficient compensation for their work

  • Orders potentially failing if the contract's ETH buffer gets depleted

  • Unfair cost distribution among users

Impact

With the protocol's keeper system, a crucial mechanism where each trade must provide adequate compensation (minimum 0.002 ETH) to incentivize keepers to execute orders. However, the GmxProxy contract has a subtle flaw in its economic assumptions.

When a user creates an order through GmxProxy, they're expected to provide execution fees through msg.value. The contract attempts to validate this using:

require(address(this).balance >= positionExecutionFee, "insufficient eth balance");

Instead of checking the user's actual contribution (msg.value), the contract looks at its total ETH balance. This means a user could submit an order with just 0.001 ETH, piggybacking on the contract's existing balance from previous operations.

The consequences are precise and measurable:

  • Keepers expect 0.002 ETH minimum per order but may receive as little as 0 ETH

  • The contract's ETH buffer, meant for legitimate operation costs, gets drained by underpaying users

  • Position management could fail entirely when the contract's ETH reserves deplete

It's about the fundamental breakdown of the keeper incentive system that powers GMX's decentralized order execution.

Recommendations

Validate both the minimum ETH requirement and the actual execution fee needed for each order, ensuring the keeper incentive mechanism works as intended. The interaction flow between PerpetualVault → GmxProxy → GMX remains secure and economically sound.

function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
// 🔐 Access control
require(msg.sender == perpVault, "invalid caller");
// 💰 Validate execution fees
uint256 positionExecutionFee = getExecutionGasLimit(
orderType,
orderData.callbackGasLimit
) * tx.gasprice;
// ✅ NEW: Validate minimum ETH requirement
require(msg.value >= minEth, "Fee below minimum");
// ✅ NEW: Validate actual execution fee
require(msg.value >= positionExecutionFee, "Insufficient execution fee");
// 🚦 Feature gate check
bytes32 executeOrderFeatureKey = keccak256(
abi.encode(
EXECUTE_ORDER_FEATURE_DISABLED,
orderHandler,
orderType
)
);
require(
dataStore.getBool(executeOrderFeatureKey) == false,
"gmx execution disabled"
);
// 📤 Forward execution fee to GMX
gExchangeRouter.sendWnt{value: positionExecutionFee}(
orderVault,
positionExecutionFee
);
// 🔄 Handle collateral transfers
if (orderType == Order.OrderType.MarketSwap ||
orderType == Order.OrderType.MarketIncrease) {
IERC20(orderData.initialCollateralToken).safeApprove(
address(gmxRouter),
orderData.amountIn
);
gExchangeRouter.sendTokens(
orderData.initialCollateralToken,
orderVault,
orderData.amountIn
);
}
// 📋 Create order parameters
CreateOrderParams memory params = _createOrderParams(
orderType,
orderData,
positionExecutionFee
);
// ✍️ Submit order to GMX
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey;
return requestKey;
}
Updates

Lead Judging Commences

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

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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

Give us feedback!