Steadefi

Summary

Using a wrong hash when depositing into GMX Market will potentially stop all the deposits from GMXVaults, based on GMX’s deposit notes.

https://github.com/gmx-io/gmx-synthetics#deposit-notes

Which states:

• Deposits are not allowed above the MAX_PNL_FACTOR_FOR_DEPOSITS

Vulnerability Details

The vulnerability lies in the fact that when either GMXVault::deposit and GMXVault::rebalanceAdd are called wrong pnlFactor (MAX_PNL_FACTOR_FOR_WITHDRAWALS) will be passed to the oracle function GMXOracle::getLpTokenValue which is intended to fetch the price of the market token when deposit and withdrawal functions are called.

Impact

As you can see in every time when the minimum market slippage amount is calculated pnl factor for withdrawals will be used:

Problem occurs when both MAX_PNL_FACTOR_FOR_DEPOSITS and MAX_PNL_FACTOR_FOR_WITHDRAWALS have different values.

There are 2 possible scenarios:

1. MAX_PNL_FACTOR_FOR_WITHDRAWALS is less than MAX_PNL_FACTOR_FOR_DEPOSITS

In this case, when the user wants to deposit the maximum allowed amount based on MAX_PNL_FACTOR_FOR_DEPOSITS transaction will most likely revert because there will be a different price of lpToken returned from the GMXOracle called with the pnlFactor = MAX_PNL_FACTOR_FOR_WITHDRAWALS, instead of the one for deposits.

1. MAX_PNL_FACTOR_FOR_WITHDRAWALS is more than MAX_PNL_FACTOR_FOR_DEPOSITS

In this case, GMXMarket’s Reader contract will return better price of the market token for the user, allowing him to deposit more than the actual value of MAX_PNL_FACTOR_FOR_DEPOSIT.

Tools Used

Recommendations

Change the isDeposit to true argument passed in the following functions: GMXDeposit::deposit and GMXRebalance::rebalanceAdd