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

Closed state is not final

NeverGonnaGiveYulUp

Summary

Status Close is not a final, one way action, even though it should be according to the specification. This enables a reopening of the vault even though it has been closed or a temporary DOS of withdrawals due to a keeper setting the state to Pause

Vulnerability Details

The function GMXEmergency#emergencyPause can be called regardless of the state that the contract is in:

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();
}

So even though it is stated in the docs that emergencyClose is a one-way irreversible action and meant to close [the] vault for good (Link). The status could be set to Paused and then Open again through the pause-resume flow (emergencyPause -> emergencyResume -> processEmergencyResume).

Impact

A vault can be reopened even though it should not be able to. Due to the repayment of borrowed funds after closing, this would remove all leverage from the vault. Additionally, a malicious keeper could cause a temporary DOS by setting the state to Pause, as emergencyWithdrawals can only happen when closed.

Tools Used

Manual Review

Recommendations

Dont allow emergencyPause to be called for any state

Updates

Lead Judging Commences

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

Vault can be opened again after close

Support

FAQs

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