Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Uninitialized PerpVault Address During Deployment

eta

Summary

The GmxProxy contract contains a critical issue where the perpVault address is not initialized during contract deployment. This leads to functions such as createOrder , settle, afterOrderExecution etc., failing for all users, as it requires the caller or the executor to be the perpVault, which is not set in the contract initialization.

Vulnerability Details

The createOrder, settle, cancelOrder, and claimCollateralRebates function check that the caller is the perpVault address, but this address is not initialized in the contract’s initialize function. As a result, the contract will always fail the check when the createOrder function is called, because the default value for an uninitialized address in Solidity is address(0).

contracts/GmxProxy.sol:initialize:#L92

// @audit initialize function is missing the perpVault parameter
function initialize(
address _orderHandler,
address _liquidationHandler,
// ... other parameters ...
) external initializer {
//@audit perpVault is not initialized
}

contracts/GmxProxy.sol:createOrder#L375

function createOrder(...) public returns (bytes32) {
require(msg.sender == perpVault, "invalid caller"); // perpVault is uninitialized
// ...
}

Impact

  1. Functional Blockage:

    • All calls to the functions such as createOrder, settle, cancelOrder, and claimCollateralRebates will fail because the perpVault is not initialized. The check msg.sender == perpVault will always revert as perpVault defaults to address(0).

  2. Security Risk:

    • The uninitialized perpVault could lead to an incomplete or failed deployment process, forcing the contract to be redeployed or modified.

POC
​
contract GmxProxyTest {
GmxProxy public gmxProxy;
address public mockPerpVault;
function setUp() public {
// 1. Deploy contract
gmxProxy = new GmxProxy();
mockPerpVault = address(new MockPerpVault());
// 2. Initialize other parameters
gmxProxy.initialize(
address(1), // orderHandler
address(2), // liquidationHandler
address(3), // adlHandler
// ... other parameters ...
);
// 3. Attempt to create an order
vm.startPrank(mockPerpVault);
vm.expectRevert("invalid caller");
gmxProxy.createOrder(
Order.OrderType.MarketIncrease,
orderData
);
vm.stopPrank();
}
}

Recommendations

It is recommended to modify the initialize function to include perpVault address and add the missing perpVault parameter to the initialize function and initialize it properly.

function initialize(
address _orderHandler,
address _liquidationHandler,
address _adlHandler,
address _gExchangeRouter,
address _gmxRouter,
address _dataStore,
address _orderVault,
address _gmxReader,
address _referralStorage,
+ address _perpVault // @audit Add perpVault parameter
) external initializer {
__Ownable2Step_init();
orderHandler = _orderHandler;
liquidationHandler = _liquidationHandler;
adlHandler = _adlHandler;
gExchangeRouter = IExchangeRouter(_gExchangeRouter);
gmxRouter = _gmxRouter;
dataStore = IDataStore(_dataStore);
orderVault = _orderVault;
gmxReader = IGmxReader(_gmxReader);
referralStorage = _referralStorage;
+ perpVault = _perpVault; // @audit Initialize perpVault
minEth = 0.002 ether;
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Admin is trusted / Malicious keepers

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Admin is trusted / Malicious keepers

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!