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

Missing Slippage Protection in Position Creation Functions ( Zero MinOutputAmount Vulnerability Analysis)

Overview

A vulnerability has been identified in the GMX protocol's position management system where both
_createIncreasePosition and _createDecreasePosition functions lack proper slippage protection. This is due to the
minOutputAmount parameter being set to zero, which could lead to substantial losses for users during volatile market
conditions.
it's crucial to implement a non-zero minOutputAmount that guarantees users receive at least a certain percentage of their
input, thereby safeguarding against adverse market conditions.

Technical Details

Affected Functions

  1. _createIncreasePosition():

  2. _createDecreasePosition():

Vulnerability Context

The GMX protocol uses a keeper system for executing position changes. When users request position modifications, these
requests are queued and later executed by keepers. The time delay between request submission and execution makes proper
slippage protection crucial.

Code Analysis

Increase Position Function

function _createIncreasePosition(bool _isLong, uint256 acceptablePrice, MarketPrices memory prices) internal {
// ... existing position size and amount calculations ...
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
// ... other parameters ...
minOutputAmount: 0 // Critical: No slippage protection
});
gmxProxy.createOrder(orderType, orderData);
}

Key Issues:

  1. Zero minOutputAmount means no minimum guarantee for position size

  2. Users could receive significantly smaller positions than expected

  3. No protection against price manipulation during the keeper execution delay

Decrease Position Function

function _createDecreasePosition(
uint256 collateralDeltaAmount,
uint256 sizeDeltaInUsd,
bool _isLong,
uint256 acceptablePrice,
MarketPrices memory prices
) internal {
// ... position size calculations ...
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
// ... other parameters ...
minOutputAmount: 0 // Critical: No slippage protection
});
gmxProxy.createOrder(orderType, orderData);
}

Key Issues:

  1. Zero minOutputAmount means no minimum guarantee for withdrawal amount

  2. Users could receive significantly less collateral than expected

  3. Particularly dangerous during market volatility or low liquidity conditions

Impact Analysis

Severity: Medium

  • Likelihood: High (Every position creation/modification is affected)

  • Impact: Medium (Financial losses possible but limited by market conditions)

Attack Scenarios

Volatile Market Scenario

1. User attempts to close position during high volatility
2. Zero minOutputAmount allows execution at any price
3. Position closes at extremely unfavorable price
4. User receives minimal collateral back

Financial Risk Assessment

  1. Position Increase Risk:

  • Potential loss: (Expected Position Size - Actual Position Size)

  • Risk factors: Market volatility, keeper delay, gas prices

  1. Position Decrease Risk:

  • Potential loss: (Expected Collateral - Actual Collateral Received)

  • Risk factors: Market manipulation, slippage, liquidity conditions

Recommended Fixes

1. Implement Dynamic MinOutputAmount

function _createIncreasePosition(bool _isLong, uint256 acceptablePrice, MarketPrices memory prices) internal {
uint256 amountIn = // ... existing calculations ...
// Calculate minimum acceptable position size
uint256 minOutputAmount = (amountIn * (BASIS_POINTS_DIVISOR - MAX_SLIPPAGE)) / BASIS_POINTS_DIVISOR;
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
// ... other parameters ...
minOutputAmount: minOutputAmount
});
}

2. Add Slippage Parameter

function _createDecreasePosition(
uint256 collateralDeltaAmount,
uint256 sizeDeltaInUsd,
bool _isLong,
uint256 acceptablePrice,
uint256 slippageTolerance, // New parameter
MarketPrices memory prices
) internal {
uint256 minOutputAmount = (collateralDeltaAmount * (BASIS_POINTS_DIVISOR - slippageTolerance)) / BASIS_POINTS_DIVISOR;
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
// ... other parameters ...
minOutputAmount: minOutputAmount
});
}

3. Implement Safety Checks

// Add to both functions
require(minOutputAmount > 0, "Invalid minOutputAmount");
require(minOutputAmount >= (amount * MIN_ACCEPTABLE_RATIO) / BASIS_POINTS_DIVISOR, "Slippage tolerance too high");

Implementation Guidelines

  1. Add Constants:

uint256 private constant MAX_SLIPPAGE = 1000; // 10% maximum slippage
uint256 private constant MIN_ACCEPTABLE_RATIO = 9000; // 90% minimum output
  1. Add Events:

event PositionCreated(
address indexed user,
uint256 amountIn,
uint256 minOutputAmount,
uint256 actualOutput
);
  1. Add Position Creation Safeguards:

modifier validateMinOutput(uint256 minOutputAmount, uint256 inputAmount) {
require(minOutputAmount > 0, "Zero min output");
require(minOutputAmount >= (inputAmount * MIN_ACCEPTABLE_RATIO) / BASIS_POINTS_DIVISOR, "Excessive slippage");
_;
}

POC

  1. Unit Tests:

function test_minOutputAmountVulnerability() public {
// Setup: User deposits
address alice = makeAddr("alice");
payable(alice).transfer(1 ether);
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 amount = 1e10;
depositFixture(alice,amount);
// Mock Market Prices to simulate very high slippage or adverse conditions
MarketPrices memory manipulatedPrices = mockData.getMarketPrices();
manipulatedPrices.shortTokenPrice.max = 1e6 / 1000; // Simulate a drastic price drop to showcase the vulnerability
// Attempt to create an increase position order
bytes[] memory metadata = new bytes[](1);
metadata[0] = abi.encode(manipulatedPrices.shortTokenPrice.max); // Acceptable price set very low
vm.prank(PerpetualVault(vault).keeper());
PerpetualVault(vault).run(true, false, manipulatedPrices, metadata); // Assuming 'run' calls _createIncreasePosition
// Currently there is not way to validate the min Amount sent but still i feel minAmount should have been set in the order
// and i cant verify how the gmx process the order
}

Conclusion

Implementing proper slippage protection is crucial for the GMX protocol's position management system. The recommended
changes will significantly improve user protection against adverse market conditions and potential manipulation. Regular
monitoring and adjustments to slippage parameters may be necessary based on market conditions.

Updates

Lead Judging Commences

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

invalid_gmx_increase/decrease_no_slippage

acceptablePrice does that job for increase/decrease positions. https://github.com/gmx-io/gmx-synthetics/blob/caf3dd8b51ad9ad27b0a399f668e3016fd2c14df/contracts/order/BaseOrderUtils.sol#L276C49-L276C66

Support

FAQs

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