GMX Swapper

The GMSwapper contract does not explicitly inherit the ISwapperV2 interface, which is inconsistent with the other swapper implementations.

Consider inheriting the ISwapper interface.

The CreateWithdrawalParams sets the execution fee to address(this).balance, but the entire contract’s balance was already sent to the ExchangeRouter: exchangeRouter.sendWnt{value: address(this).balance}(withdrawalVault, address(this).balance); Consequently, the execution fee for the params is always 0. This is not a large issue because GMX will automatically reset the params.executionFee to however much ETH was sent to the ExchangeRouter.

Set the executionFee directly to 0 within the params.

For the GM withdrawal to succeed, the swapper must provide a native token execution fee to GMX. The tests pass because they send value directly through the payable GMSwapper.swap function to make native token available for the execution fee. However, the swap does not actually receive any native asset from the Cauldron. Consequently, a liquidator would need to directly send ETH to the GMSwapper contract before commencing liquidation, and if this is not done atomically, that ETH is at-risk of being stolen by another user.

Clearly document this behavior and ensure ETH is sent to the swapper in the cook function prior to liquidation.

The current call to executeAtomicWithdrawal uses the GMXv2.1 CreateWithdrawalParams, but in v2.2 the params are different:

// Before
struct CreateWithdrawalParams {
        address receiver;
        address callbackContract;
        address uiFeeReceiver;
        address market;
        address[] longTokenSwapPath;
        address[] shortTokenSwapPath;
        uint256 minLongTokenAmount;
        uint256 minShortTokenAmount;
        bool shouldUnwrapNativeToken;
        uint256 executionFee;
        uint256 callbackGasLimit;
    }

// After
struct CreateWithdrawalParams {
        CreateWithdrawalParamsAddresses addresses;
        uint256 minLongTokenAmount;
        uint256 minShortTokenAmount;
        bool shouldUnwrapNativeToken;
        uint256 executionFee;
        uint256 callbackGasLimit;
        bytes32[] dataList;
    }

Consequently, this GMSwapper will not function with GMXv2.2.** **

Be aware of this limitation for when GMX upgrades.

Within the GMSwapper contract, certain tokens such as USDT need an explicit reset to zero before updating their allowance to a new nonzero value. Currently, the GMSwapper uses a check to see if the allowance is already type(uint256).max if not, it calls safeApprove(..., type(uint256).max). However, if the preceding aggregator or external call (swapDatum.to.functionCall(swapDatum.data)) modifies the allowance to a non-zero amount other than type(uint256).max, a subsequent attempt to approve type(uint256).max may fail for tokens like USDT. This mismatch can cause a DoS for users attempting to swap these tokens, since the contract is unable to re-approve them without first resetting to zero.

Replace the current approval pattern with a safe retry mechanism that explicitly handles tokens requiring a zero allowance reset before setting a new nonzero allowance. This can be done using a utility function such as safeApproveWithRetry which:

• Sets the allowance to zero (if not already zero).
• Approves the target address for the desired type(uint256).max amount.

Inside the GMSwapper contract, each swapData[i] entry specifies a target contract (swapDatum.to) and a arbitrary call data (swapDatum.data). When the code executes:

swapDatum.to.functionCall(swapDatum.data);

that call can perform any function call on swapDatum.to. An attacker could exploit this call to grant allowances from the GMSwapper to himself, allowing him to transfer tokens out of the GMSwapper’s balance. In future swap calls if the attacker receives the control during the flow, he could drain all the tokens remaining (just the remainingBalance, more would cause a revert) in the GMSwapper contract before the different legit swapDatum.to.functionCall(swapDatum.data) calls are completed.

Considering adding the nonReentrant modifier to the swap function.

Within the GMSwapper contract’s loop over swapData[], the function calls:

swapDatum.to.functionCall(swapDatum.data);

directly after withdrawing tokens from GMX’s executeAtomicWithdrawal, without knowing precisely how many tokens were actually received. As a result, the code and the actual caller can not precisely determine the exact token amount available for the swap, leaving the called aggregator contract (swapDatum.to) to infer or handle the balance in an ad-hoc manner. Therefore, this is an important limitation for users using the GMSwapper as they do not know exactly the amounts they can operate with beforehand to properly build an optimal swapData.

Inform the users of this limitation as addressing this issue would require a major update to the current GMSwapper code.

At the end of the GMSwapper.swap function, the code calculates:

extraShare = shareReturned - shareToMin;

This line risks an underflow if shareReturned (the total MIM shares actually deposited) ends up less than shareToMin (the user’s minimum expected shares). In Solidity ^0.8.x, this triggers an automatic underflow revert that would not clearly indicate that the user’s slippage tolerance was violated.

Before performing the subtraction, compare shareReturned and shareToMin. If shareReturned < shareToMin, revert with a clear revert message or custom error indicating that the slippage requirement was not met. For example:

if (shareReturned < shareToMin) {
    revert SlippageExceeded(shareReturned, shareToMin);
}
extraShare = shareReturned - shareToMin;

Within the GMSwapper contract’s swap function, there are no dedicated events emitted to indicate critical state changes such as the final shareReturned amount deposited in BentoBox or the token balances obtained after executeAtomicWithdrawal.

Consider emitting events for key steps:

• Post-withdrawal balances: Immediately after executeAtomicWithdrawal, emit an event detailing the new balance of tokens in the contract (e.g., LogTokensReceived(fromToken, amountReceived, ...).
• Final MIM shares: Once the final deposit into BentoBox completes, emit an event such as LogSwapResult(recipient, shareReturned, extraShare).

In the GMSwapper, there is a comment outlining the formula to compute the minimum execution fee for atomic withdrawals. However, this calculation is not implemented directly in any helper function. Developers or integrators must replicate the logic themselves, pulling the same parameters from dataStore and performing the required calculations.

Create a dedicated utility function that does the work directly for the users. For example:

function getMinExecutionFee(uint256 oraclePriceCount, uint256 currentGasPrice) public view returns (uint256) {
    uint256 estimatedGasLimit = dataStore.getUint(Keys.withdrawalGasLimitKey());
    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 * currentGasPrice;
}

The shareToMin parameter guarantees that when the MIM tokens are deposited after all the operations back in the degenbox on the behalf of the user, the shares received will be at least sharesToMin. While this minimizes the risk of getting sandwiched, the degenbox share price can still change due to strategy performance. For example:

• 1000 shares and 1000 assets (1:1)
• The user specifies sharesToMin = 100, which is equal to 100 assets at that time
• Before the transaction is executed, the strategy reports a loss of 500 assets and the new ratio becomes 2:1.
• Now sharesToMin = 100 is actually 50 assets.

Be sure to document this risk and enforce proper slippage values in your frontend for the swap data as well.

The CreateWithdrawalParams sets the execution fee to address(this).balance, but the entire contract’s balance was already sent to the ExchangeRouter: exchangeRouter.sendWnt{value: address(this).balance}(withdrawalVault, address(this).balance); Consequently, the execution fee for the params is always 0. This is not a large issue because GMX will automatically reset the params.executionFee to however much ETH was sent to the ExchangeRouter.

Set the executionFee directly to 0 within the params.

The swap function allows users to perform external swaps through functionCall, allowing them to set custom to and data. However, a user could exploit this to reset the maximum MIM token approval granted to the box contract in the constructor. As a result, no further swaps can be executed until the approval is restored, causing a temporary denial of service.

Consider imposing restrictions on the external swap call to prevent changes to the MIM token approval on the box contract.

The dataStore variable is assigned in the constructor but is only referenced in commented-out code.

Remove the dataStore variable if it serves no purpose.

The swap function is intended to be called atomically following Cauldron.removeCollateral (i.e., within the same transaction). However, if executed in separate transactions, any tokens sitting idle in the swapper contract become vulnerable. Additionally, swap relies on user-supplied swapData to convert withdrawn tokens into MIM. If swapData is misconfigured to receive a different token (e.g., not MIM), those tokens are left unhandled in the contract. Since swap allows arbitrary input data, a malicious user could simply craft a swapData payload to drain all tokens left behind by a prior failed or partial call.

• Document clearly that the swap function should only be used as part of an atomic flow.
• Consider reverting if MIM balance is zero after the swap (before box.deposit) to prevent misconfigured swapData from succeeding.

The getMinExecutionFee function calculates the required execution fee using user-supplied oraclePriceCount and currentGasPrice. However, if oraclePriceCount < 3, the function will underestimate the required fee. In practice, GMX always uses a minimum of 3 oracle prices for withdrawals, as enforced by GasUtils::estimateWithdrawalOraclePriceCount source and validated during fee checks in WithdrawalUtils source. Accepting values below 3 in getMinExecutionFee can mislead integrators or frontends into providing insufficient fees, resulting in failed transactions and poor UX.

Update getMinExecutionFee to validate that oraclePriceCount >= 3, or hardcode the minimum internally to align with GMX’s expectations.