Relevant GitHub Links
Summary
min max price on getMarketTokenPrice is not utilized such that deposit and withdrawal can use the same price, leading to free tx for cost-free manipulation
GMX provides getMarketTokenPrice on its synethicReader which leverages MarketUtils. It allows passing in index/long/short token price with min/max. The isDeposit flag would then be used to determine whether the min or max price would be used for calculating marketTokenPrice, this is important to always favor the protocol and prevent MEV.
However on the getMarketTokenInfo implemented in GMXOracle, it passes in the same price from the oracle to the min/max price for all long&short/lpToken. This implies the same pricing is used for both deposit and withdrawal, enabling user to freely deposit/withdraw without cost or slippage. Malicious users can use this to trigger rebalance, and hence deposit or withdrawal directly on GMX that benefit the attacker with the use of bundled tx.
plain textfunction getMarketTokenPrice( DataStore dataStore, Market.Props memory market, Price.Props memory indexTokenPrice, Price.Props memory longTokenPrice, Price.Props memory shortTokenPrice, bytes32 pnlFactorType, bool maximize ) external view returns (int256, MarketPoolValueInfo.Props memory) { return MarketUtils.getMarketTokenPrice( dataStore, market, indexTokenPrice, longTokenPrice, shortTokenPrice, pnlFactorType, maximize ); }
Vulnerability Details
Impact
free deposit and withdrawal due to the same token price is used for min or max price, which leading to the same marketTokenPrice calculation for deposit and withdrawal.