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

Acceptable Price Validation Vulnerability in PerpetualVault.sol

petargvr94

Summary

The PerpetualVault contract relies on an acceptablePrice parameter for position management operations on GMX. This value is provided by an off-chain keeper and determines the maximum or minimum price at which a position can be increased or decreased. Insufficient validation of acceptablePrice introduces a vulnerability to front-running, stale price data, and excessive slippage, potentially resulting in significant fund losses.

Vulnerability

Vulnerability Details

The contract accepts acceptablePrice as an input during position increase and decrease operations. This value is intended to protect against slippage; however, the contract lacks robust on-chain validation mechanisms for this parameter, leading to the following risks:

  • Front-running and Sandwich Attacks: An attacker can front-run a transaction, causing price movement beyond acceptablePrice, leading to failed or unfavorable orders.

  • Keeper Mistakes: Malicious or misconfigured keepers can supply overly lenient acceptablePrice values, enabling position execution at extremely unfavorable rates.

  • Stale Price Data: Off-chain data may become stale, leading to position adjustments based on outdated pricing, amplifying slippage losses.

  • Over-reliance on Off-Chain Data: Heavy dependence on an off-chain keeper increases the risk of incorrect or malicious data being used for critical pricing decisions.

Affected Functions:

  • _createIncreasePosition()

  • _createDecreasePosition()

  • run()

  • runNextAction()

Impact

The absence of proper validation on acceptablePrice exposes users to the following threats:

  • Loss of funds due to excessive slippage.

  • Potential exploitation via front-running, causing positions to be executed at unfavorable prices.

  • Keeper compromise resulting in deliberate execution of orders at poor rates.

  • Execution based on stale pricing data, increasing the likelihood of executing orders at undesirable market conditions.

Tools Used

Manual Code Review

Recommendations

1. Implement On-Chain Slippage Bounds

Introduce a maximum allowable slippage parameter to prevent acceptablePrice from deviating excessively from the current market price.

uint256 public maxSlippageBps = 500; // Example: 5%
function _validateSlippage(uint256 acceptablePrice, uint256 currentPrice, bool isLong) internal view {
uint256 slippageLimit = (currentPrice * maxSlippageBps) / BASIS_POINTS_DIVISOR;
if (isLong && acceptablePrice > currentPrice + slippageLimit) {
revert Error.UnacceptableSlippage();
}
if (!isLong && acceptablePrice < currentPrice - slippageLimit) {
revert Error.UnacceptableSlippage();
}
}

2. Validate Price Freshness

Ensure that price data passed from off-chain sources is recent by introducing this function:

function _validateFreshPrice(uint256 timestamp) internal view {
if (block.timestamp > timestamp + 2 minutes) {
revert Error.StalePriceData();
}
}

Updates

Lead Judging Commences

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

Admin is trusted / Malicious keepers

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. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."

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.