Liquidity Management

Summary

Because the validCallback modifier in GmxProxy only whitelists a single set of handlers, callbacks from a second OrderHandler cause the contract to revert—this breaks the deposit flow (since afterOrderExecution() never runs), leaving users’ funds deposited on GMX without receiving corresponding shares, ultimately resulting in lost funds.

Vulnerability Details

GmxProxy.sol recieves GMX callbacks via the afterOrderExecution() function. To avoid malicious callbacks from positions that don't belong to the PerpetualVault, the validCallback modifier is used:

According to the GMX documentation:

When creating a callback contract, the callback contract may need to whitelist the DepositHandler, OrderHandler or WithdrawalHandler, it should be noted that new versions of these handlers may be deployed as new code is added to the handlers, it is also possible for two handlers to temporarily exist at the same time, e.g. OrderHandler(1), OrderHandler(2), due to this, the callback contract should be able to whitelist and simultaneously accept callbacks from multiple DepositHandlers, OrderHandlers and WithdrawalHandlers

If there are two OrderHandlers, the GMXProxy will always revert when receiving callbacks from one of the two OrderHandlers.

This causes huge problems because the order will go through on the GMX side, but afterOrderExecution() won't run in the PerpetualVault.

During deposits to an open position, for example, shares are minted in afterOrderExecution, so the depositors funds will be add to the position, but they won't receive their shares causing loss of funds.

Impact

Loss of funds

Tools Used

Manual review

Recommendations

Allow for handling of multiple OrderHandlers.