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

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/oracles/GMXOracle.sol#L167-L176

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.

    function 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
            );
    }

https://github.com/gmx-io/gmx-synthetics/blob/613c72003eafe21f8f80ea951efd14e366fe3a31/contracts/reader/Reader.sol#L187-L206

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.

Tools Used

Recommendations

consider adding a small fee(5bps) to buffer the price returned from _getTokenPriceMinMaxFormatted on both sides.