IllIllI - Slippage is not respected if PnL swap associated with a decrease order fails

[sherlock-admin]

IllIllI

medium

Slippage is not respected if PnL swap associated with a decrease order fails

Summary

After a position has been decreased, the user has an option to convert the PnL token to any other token via a swap.

Vulnerability Detail

If the swap fails (which is where the slippage is checked), the tokens are sent directly to the user, without checking whether there was slippage

Impact

A user will get back fewer tokens than they expect, if there was a large price impact, and the subsequent swap fails due to e.g. the market being temporarily disabled, or the swap impact being too large, or a token being swapped through is paused

Code Snippet

Funds are sent back directly, without checking for slippage:

// File: gmx-synthetics/contracts/order/DecreaseOrderUtils.sol : DecreaseOrderUtils._handleSwapError()   #1

168            emit SwapUtils.SwapReverted(reason, reasonBytes);
169    
170            MarketToken(payable(order.market())).transferOut(
171                result.outputToken,
172                order.receiver(),
173 @>             result.outputAmount,
174                order.shouldUnwrapNativeToken()
175            );
176:       }

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/order/DecreaseOrderUtils.sol#L168-L176

Tool used

Manual Review

Recommendation

Calculate whether the USD value of outputToken is equivalent to the minOutputAmount expected by the order, and revert if it's less than required

[xvi10]

Fix in gmx-io/gmx-synthetics#108