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

Incorrect Token Approval in `createOrder` Function Leads to Failed Token Transfers

Summary

The createOrder function in the GmxProxy contract incorrectly approves the gmxRouter instead of the gExchangeRouter for token transfers. This leads to failed token transfers when creating orders, causing orders to lack collateral and resulting in failed transactions or stuck funds.

Vulnerability Details

In the createOrder function, the contract approves the gmxRouter to spend the initialCollateralToken. However, the actual token transfer is performed by the gExchangeRouter using the sendTokens function. This discrepancy causes the token transfer to fail due to lack of approval, leading to insufficient collateral for the created orders.

Root Cause:
The createOrder function incorrectly approves the gmxRouter instead of the gExchangeRouter for token transfers.

Proof of Concept:

  1. Incorrect Approval:

    IERC20(orderData.initialCollateralToken).safeApprove(
    address(gmxRouter), // Incorrect approval
    orderData.amountIn
    );
  2. Token Transfer:

    gExchangeRouter.sendTokens(
    orderData.initialCollateralToken,
    orderVault,
    orderData.amountIn
    );
  3. Impact Demonstration:

    • When the createOrder function is called with orderType as MarketSwap or MarketIncrease, the approval is given to gmxRouter.

    • The sendTokens function call by gExchangeRouter will fail due to lack of approval, causing the order to lack collateral and fail.

** **

IERC20(orderData.initialCollateralToken).safeApprove(
address(gExchangeRouter), // Correct approval
orderData.amountIn
);

Impact

Orders will fail due to insufficient collateral, leading to loss of user funds as tokens are not properly transferred to GMX's order vault. This can result in failed transactions or stuck funds.

Tools Used

Manual

Recommendations

To fix this issue, the approval should be given to the gExchangeRouter instead of the gmxRouter

/**
* @notice Creates an order.
* @dev This function requires the receipient to be the perpetual vault and ensures sufficient ETH balance for the execution fee.
* It handles token approvals, transfers, and constructs the order parameters before creating the order via `gExchangeRouter`.
* @param orderType The type of the order (e.g., MarketIncrease, MarketDecrease, etc.).
* @param orderData The data associated with the order.
* @return The request key of the created order.
*/
function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
uint256 positionExecutionFee = getExecutionGasLimit(
orderType,
orderData.callbackGasLimit
) * tx.gasprice;
require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);
// check if execution feature is enabled
bytes32 executeOrderFeatureKey = keccak256(
abi.encode(
EXECUTE_ORDER_FEATURE_DISABLED,
orderHandler,
orderType
)
);
require(
dataStore.getBool(executeOrderFeatureKey) == false,
"gmx execution disabled"
);
gExchangeRouter.sendWnt{value: positionExecutionFee}(
orderVault,
positionExecutionFee
);
if (
orderType == Order.OrderType.MarketSwap ||
orderType == Order.OrderType.MarketIncrease
) {
IERC20(orderData.initialCollateralToken).safeApprove(
- address(gmxRouter),
+ address(gExchangeRouter),
orderData.amountIn
);
gExchangeRouter.sendTokens(
orderData.initialCollateralToken,
orderVault,
orderData.amountIn
);
}
CreateOrderParamsAddresses memory paramsAddresses = CreateOrderParamsAddresses({
receiver: perpVault,
cancellationReceiver: address(perpVault),
callbackContract: address(this),
uiFeeReceiver: address(0),
market: orderData.market,
initialCollateralToken: orderData.initialCollateralToken,
swapPath: orderData.swapPath
});
CreateOrderParamsNumbers memory paramsNumber = CreateOrderParamsNumbers({
sizeDeltaUsd: orderData.sizeDeltaUsd,
initialCollateralDeltaAmount: orderData.initialCollateralDeltaAmount,
triggerPrice: 0,
acceptablePrice: orderData.acceptablePrice,
executionFee: positionExecutionFee,
callbackGasLimit: orderData.callbackGasLimit,
minOutputAmount: orderData.minOutputAmount, // this param is used when swapping. is not used in opening position even though swap involved.
validFromTime: 0
});
CreateOrderParams memory params = CreateOrderParams({
addresses: paramsAddresses,
numbers: paramsNumber,
orderType: orderType,
decreasePositionSwapType: Order
.DecreasePositionSwapType
.SwapPnlTokenToCollateralToken,
isLong: orderData.isLong,
shouldUnwrapNativeToken: false,
autoCancel: false,
referralCode: referralCode
});
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey;
return requestKey;
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

invalid_approval_gmxRouter_instead_of_gExchangeRouter

Router is the one collecting tokens: https://github.com/gmx-io/gmx-synthetics/blob/caf3dd8b51ad9ad27b0a399f668e3016fd2c14df/contracts/router/BaseRouter.sol#L46 https://github.com/gmx-io/gmx-synthetics/blob/caf3dd8b51ad9ad27b0a399f668e3016fd2c14df/contracts/router/Router.sol#L27

Support

FAQs

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

Give us feedback!