Liquidity Management

Summary

GMX doc suggest that there may be two OrderHandler contract at some point and it is necessary that the protocol will need to whitelist several at the same time. Current implentation of the GmxProxy.sol allows only one kinds of OrderHandler and may deny fallback calls.

Vulnerability Details

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 (https://github.com/gmx-io/gmx-synthetics#Integrations)

The above recommendation is not followed by the GmxProxy.sol which may lead to situations where callback calls are not accepted.

Impact

Legitimate fallback calls reverting leading to the broken flow of the system.

Tools Used

Manual review

Recommendations

Follow the official recommendagtions below:

For callback contracts instead of maintaining a separate whitelist for DepositHandlers, OrderHandlers, WithdrawalHandlers, a possible solution would be to validate the role of the msg.sender in the RoleStore, e.g. RoleStore.hasRole(msg.sender, Role.CONTROLLER), this would check that the msg.sender is a valid handler