Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

`GMXVault:emergencyPause` can prevent in flight transactions from processing

NeverGonnaGiveYulUp

Summary

Calling emergencyPause while a 2 step-process has not finished yet, can cause a loss of funds.

Vulnerability Details

The GMXVault#emergencyPause function can be called at any time and any status:

//GMXVault
function emergencyPause() external payable onlyKeeper {
GMXEmergency.emergencyPause(_store);
}
​
//GMXEmergency#emergencyPause
function emergencyPause(
GMXTypes.Store storage self
) external {
self.refundee = payable(msg.sender);
​
GMXTypes.RemoveLiquidityParams memory _rlp;
​
// Remove all of the vault's LP tokens
_rlp.lpAmt = self.lpToken.balanceOf(address(this));
_rlp.executionFee = msg.value;
​
GMXManager.removeLiquidity(
self,
_rlp
);
​
self.status = GMXTypes.Status.Paused;
​
emit EmergencyPause();
}

When a user deposits or withdraws in the vault they need to wait for GMXCallback to be called to receive either their shares in the vault or their assets they deposited. If GMXVault:emergencyPause is called during this time, the callback from GMX will not be able to run. This will cause a loss of user funds.

Impact

A user will lose either their shares in the vault or their deposited assets.

Tools Used

Manuel Review

Recommendations

Allow emergencyPause to only be called in status Open. To prevent a DOS of the emergency pause by someone spamming deposits either:

  1. use flashbots to execute the tx, so its not visible in the mempool

  2. add a boolean flag that can be set by keeper and which disallows initiating any new processes when the state is currently Open

Updates

Lead Judging Commences

hans Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Not caching the previous state on emergencyPause

Impact: High Likelihood: Low/Medium Deposit or withdraw that were in progress will be ignored and cause fund loss. Because emergencyPause is only callable by keepers, Medium is the proper severity.

Support

FAQs

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