Steadefi

Summary

lpTokenValue will be always calculated with the factor type WITHDRAWALS even if the action is deposit, due to the hardcoded false value passed in the function parameter.

Vulnerability Details

lpTokeValue function defined is GMXOracle.sol is responsible for returning the price of the GM token and it calculates this price based on the _pnlFactorType of either withdraw or deposit actions. To determine the type of action, isDeposit param is passed to the function.

But wherever this function is called the isDeposit is passed always as false which will lead to the incorrect lpTokenValue.

Proof of concept

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXDeposit.sol#L92

When user deposit lpToken to GMXVault the isDeposit is passed as false which will always lead to the incorrect price calculations.

Even in the calMinMarketSlippage this value is always hardcoded to false. This can result in the loss of user funds if the slippage is calculated on the wrong lpTokenPrice.

Impact

The function is used to calculate the value of user GM Deposit or slippage, so if the price based on MAX_PNL_FACTOR_FOR_WITHDRAWALS is low this could lead to the loss of user deposits. For example if user deposit GM token and value of it is calcualted low for withdraw but it is actually high for deposits, the user will end up with the less gvTokens(vault token).

Tools Used

manual review

Recommendations

Always passed appropriate boolean when calculating the lpTokenValue, i.e true for deposit and false for withdraw.