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

Unsafe Token Approval Pattern in createOrder Function

Deleted user

Summary

The GmxProxy contract uses an unsafe token approval pattern in the createOrder function that could lead to failed transactions with certain ERC20 tokens like USDT that require allowance to be set to 0 before updating to a new value.

Vulnerability Details

function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
// ...existing code...
if (orderType == Order.OrderType.MarketSwap ||
orderType == Order.OrderType.MarketIncrease) {
IERC20(orderData.initialCollateralToken).safeApprove(
address(gmxRouter),
orderData.amountIn
);
// ...existing code...
}
}

The safeApprove() function is called directly with a new allowance value without first resetting it to zero. Some ERC20 tokens (notably USDT) implement strict requirements where:

  1. If current allowance is not zero, you must first set it to zero

  2. Only then can you set it to a new non-zero value

This implementation pattern can cause transactions to revert when:

  • There's an existing non-zero allowance

  • The token implements strict allowance checks

  • Multiple orders are created with different allowance amounts

Impact

  • Severity: High

  • Likelihood: High for tokens like USDT

  • Impact: Critical as it can block core protocol functionality

The issue could:

  1. Cause order creation to fail

  2. Block users from opening positions

  3. Require manual intervention to reset approvals

Tools Used

  • Manual code review

  • Solidity visual developer

  • Historical examples of similar issues with USDT

Recommendations

Implement a safe approval pattern that works with all ERC20 tokens:

function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
// ...existing code...
if (orderType == Order.OrderType.MarketSwap ||
orderType == Order.OrderType.MarketIncrease) {
IERC20 token = IERC20(orderData.initialCollateralToken);
// First reset approval to 0
uint256 currentAllowance = token.allowance(address(this), address(gmxRouter));
if (currentAllowance > 0) {
token.safeApprove(address(gmxRouter), 0);
}
// Then set the new approval amount
token.safeApprove(address(gmxRouter), orderData.amountIn);
// ...existing code...
}
}

Alternatively, consider using OpenZeppelin's safeIncreaseAllowance():

import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Permit.sol";
​
function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
// ...existing code...
if (orderType == Order.OrderType.MarketSwap ||
orderType == Order.OrderType.MarketIncrease) {
IERC20(orderData.initialCollateralToken).safeIncreaseAllowance(
address(gmxRouter),
orderData.amountIn
);
// ...existing code...
}
}
Updates

Lead Judging Commences

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

invalid_safeApprove_no_reset

USDT or other unusual ERC20 tokens: out of scope. For the other reports: No proof that the allowance won't be consumed by the receiver.

Support

FAQs

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

Give us feedback!