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

Vault cant retry failed GMX swaps due to safeApprove() in GmxProxy.sol::createOrder().

wickie

Summary

The Gamma Vault uses Paraswap and GMX swap functionalities to swap from collateral token to index token and vice versa. Before the GMX swap order is requested, the GmxProxy approve the swap token to GMX exchange router via safeApprove(). If the order fails for some reason, the vault handles it in PerpetualVault.sol::afterOrderCancellation() by setting the nextAction to SWAP_ACTION for the keeper to execute via runNextAction(). This will revert as safeApprove() will revert due to the allowance being non-zero.

Vulnerability Details

Before an order is created in GmxProxy::createOrder(),

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),
orderData.amountIn
);
....

This is fine when orders are successfully, as the allowance is set back to 0. But when a swap fails, the approval transactions still go through while the proxy wait for GMX to callback. If an order is cancelled for some reason, GMX will call afterOrderCancellation(), which will call PerpetualVault.sol::afterOrderCancellation().

function afterOrderCancellation(
bytes32 key,
Order.Props memory order,
EventLogData memory /* eventData */
) external override validCallback(key, order) {
IGmxProxy.OrderResultData memory orderResultData = IGmxProxy
.OrderResultData(
order.numbers.orderType,
order.flags.isLong,
order.numbers.sizeDeltaUsd,
address(0),
0,
queue.isSettle
);
IPerpetualVault(perpVault).afterOrderCancellation(
key,
order.numbers.orderType,
orderResultData
);
delete queue;
}

In PerpetualVault.sol::afterOrderCancellation(), OrderType is MarketSwap and NextActionSelector will be SWAP_ACTION.

function afterOrderCancellation(
bytes32 requestKey,
Order.OrderType orderType,
IGmxProxy.OrderResultData memory orderResultData
) external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
​
if (orderResultData.isSettle) {
nextAction.selector = NextActionSelector.SETTLE_ACTION;
} else if (orderType == Order.OrderType.MarketSwap) {
// If GMX swap fails, retry in the next action.
@> nextAction.selector = NextActionSelector.SWAP_ACTION;
nextAction.data = abi.encode(swapProgressData.remaining, swapProgressData.isCollateralToIndex);
} else {
....

This do a GMX swap or a Paraswap or both based on the keeper's input in runNextAction(). If it makes a GMX swap via runSwap() -> _doGmxSwap(), GmxProxy.sol::createOrder() will revert due to safeApprove().

function safeApprove(IERC20 token, address spender, uint256 value) internal {
// safeApprove should only be called when setting an initial allowance,
// or when resetting it to zero. To increase and decrease it, use
// 'safeIncreaseAllowance' and 'safeDecreaseAllowance'
require(
@> (value == 0) || (token.allowance(address(this), spender) == 0),
"SafeERC20: approve from non-zero to non-zero allowance"
);
_callOptionalReturn(token, abi.encodeWithSelector(token.approve.selector, spender, value));

Impact

The Vault cant retry GMX swaps if they fail.

Tools Used

Manual Review

Recommendations

Reset safeApprove to 0 after order creation.

Updates

Lead Judging Commences

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

finding_MarketSwap_cancelled_DoS_protocol_with_safeApprove_no_reset

Likelihood: Medium/High, when MarketSwap order are canceled. Impact: High, DoS MarketSwap order, safeApprove reverting.

Appeal created

vinica_boy Auditor
8 months ago
n0kto Lead Judge
8 months ago
n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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