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

Incorrect calculation of execution fee leading to revert for gmx swap

vinica_boy

Summary

Current implementation of gas execution fee is deviating from the implementation on GMX which will lead to insufficient amount of execution fee sent for GMX swap orders.

Vulnerability Details

In GMX OrderHandler the provided execution fee is verified against a certain value :
https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/exchange/OrderHandler.sol#L108

function validateExecutionFee(DataStore dataStore, uint256 estimatedGasLimit, uint256 executionFee, uint256 oraclePriceCount) internal view {
uint256 gasLimit = adjustGasLimitForEstimate(dataStore, estimatedGasLimit, oraclePriceCount);
uint256 minExecutionFee = gasLimit * tx.gasprice;
if (executionFee < minExecutionFee) {
revert Errors.InsufficientExecutionFee(minExecutionFee, executionFee);
}
}
function adjustGasLimitForEstimate(DataStore dataStore, uint256 estimatedGasLimit, uint256 oraclePriceCount) internal view returns (uint256) {
uint256 baseGasLimit = dataStore.getUint(Keys.ESTIMATED_GAS_FEE_BASE_AMOUNT_V2_1);
baseGasLimit += dataStore.getUint(Keys.ESTIMATED_GAS_FEE_PER_ORACLE_PRICE) * oraclePriceCount;
uint256 multiplierFactor = dataStore.getUint(Keys.ESTIMATED_GAS_FEE_MULTIPLIER_FACTOR);
uint256 gasLimit = baseGasLimit + Precision.applyFactor(estimatedGasLimit, multiplierFactor);
return gasLimit;
}

Where estimatedGasLimit is calculated as follows:

function estimateExecuteSwapOrderGasLimit(DataStore dataStore, Order.Props memory order) internal view returns (uint256) {
uint256 gasPerSwap = dataStore.getUint(Keys.singleSwapGasLimitKey());
return dataStore.getUint(Keys.swapOrderGasLimitKey()) + gasPerSwap * order.swapPath().length + order.callbackGasLimit();
}

The formula can be represented as: baseGasLimit + gasFeePerOracle * oraclePriceCount + estimatedGasLimit * multiplierFactor .

We have the same calculation in GmxProxy::getExecutionGasLimit():

executionGasLimit =
baseGasLimit +
((estimatedGasLimit + _callbackGasLimit) * multiplierFactor) /
PRECISION;

The difference comes from the estimatedGasLimit calculations:

In GMX it is calculated as swapOrderGasLimit + gasPerSwap * swapPathLength + callbackGasLimit but in Gamma we have swapOrderGasLimit + gasPerSwap + callbackGasLimit. Which means that for swap path with length > 1 GMX swap orders will always revert. The other consequence of current implementation would be that execution fee for other types of order would be slightly higher than what is needed.

Impact

DoS for GMX swap orders

Tools Used

Manual review.

Recommendations

Consider implementing execution fee calculations 1:1 as in GMX to avoid DoS scenarios.

Updates

Lead Judging Commences

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

finding_swapPath_does_not_increase_the_executionFee

Likelihood: Low/Medium, when swapPath has more than 1 item. Impact: Medium/High, could lead to not enough fee collected to execute the transaction in GMX

Support

FAQs

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