sherlock-audit/2023-02-gmx-judging

IllIllI - ADL operations do not have any slippage protection

Opened this issue · 1 comments

IllIllI

medium

ADL operations do not have any slippage protection

Summary

ADL operations do not have any slippage protection

Vulnerability Detail

ADL orders, generated by the ADL keeper, are forced to allow unlimited slippage, which means the user may get a lot less than they deserve.

Impact

There may be a large price impact associated with the ADL order, causing the user's large gain to become a loss, if the price impact factor is large enough, which may be the case if the price suddenly spikes up a lot, and there are many ADL operations after a lot of users exit their positions.

Code Snippet

Any price is allowed, and the minimum output amount is set to zero:

// File: gmx-synthetics/contracts/adl/AdlUtils.sol : AdlUtils.createAdlOrder()   #1

141    
142            Order.Numbers memory numbers = Order.Numbers(
143                Order.OrderType.MarketDecrease, // orderType
144                Order.DecreasePositionSwapType.NoSwap, // decreasePositionSwapType
145                params.sizeDeltaUsd, // sizeDeltaUsd
146                0, // initialCollateralDeltaAmount
147                0, // triggerPrice
148 @>             position.isLong() ? 0 : type(uint256).max, // acceptablePrice
149                0, // executionFee
150                0, // callbackGasLimit
151 @>             0, // minOutputAmount
152                params.updatedAtBlock // updatedAtBlock
153:           );

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/adl/AdlUtils.sol#L141-L153

The only checks after the ADL order completes are related to global metrics, not position-specific ones.

Tool used

Manual Review

Recommendation

Introduce a MAX_POSITION_IMPACT_FACTOR_FOR_ADL, similar to MAX_POSITION_IMPACT_FACTOR_FOR_LIQUIDATIONS

xvi10 commented

The maximum price impact is capped by the MAX_POSITION_IMPACT_FACTOR value