Steadefi

Summary

Following the deposit/withdraw flows with the before/after checkings, there are currently no verifications that ensure deposits/withdrawals would no go above the Max PnL factor.

Vulnerability Details

In this two-step transaction process (where GMX is based for various asset transfer transactions), there are multiple junctures where potential reversals could occur, necessitating the Vault's ability to manage them appropriately.

These can be categorized at a glance in before and after checkings and are mostly contained in GMXChecks.sol:

For example, consider this beforeDeposit check that verifies if the user did not send enough execution fee:

https://github.com/Cyfrin/2023-10-SteadeFi/blob/main/contracts/strategy/gmx/GMXChecks.sol#L62-L63

The issue arises precisely when executing deposits or withdrawals in this two-step flow and going for their respective checks under beforeDepositChecks and beforeWithdrawChecks, there are currently no verifications in place that a deposit should not go above the Max PnL factor for deposits, and at the same time there is no check available for verifying that a withdrawal should not go above Max PnL factor for withdrawals.

Nevertheless, as this flag/key of Max PnL factor is currently not defined in the codebase, and the verifications missing require this factor/cap to be established in the first place, a suggestion on how to calculate it can be found in https://blog.whitebit.com/en/what-is-profit-and-loss/. Also a good reference (which is referring to GMX) is https://www.collider.vc/post/gmx-granted-million-dollar-bug-bounty-to-collider-the-bug-aftermath

Impact

As the market token price is influenced by both the total value of assets within the pool and the aggregate pending profit and loss of traders' active positions, it is necesary to establish the necessary caps used to calculate the market token price depending if it is a deposit or a withdrawal.

Tools Used

Manual Review

Recommendations

Establish the proper PnL factor caps when calculating the market token price for deposits/withdrawals and ensure the verifications are in place under beforeDepositChecks and beforeWithdrawChecks in GMXChecks.sol