Summary
PerpVault does not estimate market swap properly as is prescribed in GMX, which could lead to failed transactiojn on GMX.
Vulnerability Details
When estimating the market swap order it simply does this.
function getExecutionGasLimit(
Order.OrderType orderType,
uint256 _callbackGasLimit
) public view returns (uint256 executionGasLimit) {
uint256 baseGasLimit = dataStore.getUint(
ESTIMATED_GAS_FEE_BASE_AMOUNT_V2_1
);
uint256 oraclePriceCount = 5;
baseGasLimit +=
dataStore.getUint(ESTIMATED_GAS_FEE_PER_ORACLE_PRICE) *
oraclePriceCount;
uint256 multiplierFactor = dataStore.getUint(
ESTIMATED_GAS_FEE_MULTIPLIER_FACTOR
);
uint256 gasPerSwap = dataStore.getUint(SINGLE_SWAP_GAS_LIMIT);
uint256 estimatedGasLimit;
if (orderType == Order.OrderType.MarketSwap) {
estimatedGasLimit =
dataStore.getUint(SWAP_ORDER_GAS_LIMIT) +
gasPerSwap;
}
executionGasLimit =
baseGasLimit +
((estimatedGasLimit + _callbackGasLimit) * multiplierFactor) /
PRECISION;
}
Estimated Gas is calculated as so: estimatedGasLimit = dataStore.getUint(SWAP_ORDER_GAS_LIMIT) + gasPerSwap; on gamma, while in GMX estimated gas for market swap is multiplied by swap paths, as supplied by the keeper on run.
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();
}
So the actual swapPath is only known at execution as supplied by metadata when the keeper executes run.
function run(
bool isOpen,
bool isLong,
MarketPrices memory prices,
bytes[] memory metadata
) external nonReentrant {
_noneFlow();
_onlyKeeper();
if (gmxProxy.lowerThanMinEth()) {
revert Error.LowerThanMinEth();
}
flow = FLOW.SIGNAL_CHANGE;
if (isOpen) {
if (positionIsClosed) {
if (_isFundIdle() == false) {
revert Error.InsufficientAmount();
}
if (_isLongOneLeverage(isLong)) {
_runSwap(metadata, true, prices);
} else {
(uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
_createIncreasePosition(isLong, acceptablePrice, prices);
}
} else {
if (beenLong == isLong) {
revert Error.NoAction();
} else {
nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(isLong);
if (_isLongOneLeverage(beenLong)) {
_runSwap(metadata, false, prices);
} else {
(uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
_createDecreasePosition(0, 0, beenLong, acceptablePrice, prices);
}
}
}
} else {
if (positionIsClosed == false) {
if (_isLongOneLeverage(beenLong)) {
_runSwap(metadata, false, prices);
} else {
(uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
_createDecreasePosition(0, 0, beenLong, acceptablePrice, prices);
}
} else {
revert Error.NoAction();
}
}
}
Impact
Under estimation of gas could lead to failure of transactions on GMX.
Tools Used
manual
Recommendations
Market should update the estimation of market swaps, to include swap parts, so as not to underestimate market swaps which could possibly lead to failure of the order type on gmx.
