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

Emergency Closed Vault Can Be Paused Then Resume

nisedo

Vulnerability Details

The emergencyClose function is intended to be a final measure to repay all debts and shut down the vault permanently, as indicated by the function's documentation. This action should be irreversible to ensure the finality and security of the vault's emergency closure process.

File: GMXVaul.sol
/**
* @notice Repays all debt owed by vault and shut down vault, allowing emergency withdrawals
* @dev Note that this is a one-way irreversible action
* @dev Should be called by approved Owner (Timelock + MultiSig)
* @param deadline Timestamp of swap deadline
*/
function emergencyClose(uint256 deadline) external onlyOwner {
GMXEmergency.emergencyClose(_store, deadline);
}

However, a pathway exists to effectively reopen a vault after it has been closed using emergencyClose by invoking the emergencyPause and emergencyResume functions. These functions alter the vault's status, allowing for the resumption of operations which contradicts the intended irreversible nature of an emergency close.

File: GMXEmergency.sol
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();
}
File: GMXEmergency.sol
function emergencyResume(
GMXTypes.Store storage self
) external {
GMXChecks.beforeEmergencyResumeChecks(self);
​
​
self.status = GMXTypes.Status.Resume;
​
​
self.refundee = payable(msg.sender);
​
​
GMXTypes.AddLiquidityParams memory _alp;
​
​
_alp.tokenAAmt = self.tokenA.balanceOf(address(this));
_alp.tokenBAmt = self.tokenB.balanceOf(address(this));
_alp.executionFee = msg.value;
​
​
GMXManager.addLiquidity(
self,
_alp
);
}

Impact

The impact of this finding is significant, as it undermines the trust model of the emergency close process. Users and stakeholders expect that once a vault is closed in an emergency, it will remain closed as a protective measure. The ability to resume operations after an emergency closure could expose the vault to additional risks and potentially be exploited by malicious actors, especially if the original closure was due to a security threat.

PoC

Add this to GMXEmergencyTest.t.sol and test with forge test --mt test_close_then_pause -vv:

function test_close_then_pause() external {
// Pause the vault
vault.emergencyPause();
console2.log("vault status", uint256(vault.store().status));
​
// Close the vault
vault.emergencyClose(deadline);
console2.log("vault status", uint256(vault.store().status));
​
// Pause the vault again
vault.emergencyPause();
console2.log("vault status", uint256(vault.store().status));
assertEq(uint256(vault.store().status), 10, "vault status not set to paused");
​
// Resume the vault
vault.emergencyResume();
console2.log("vault status", uint256(vault.store().status));
}

Recommendations

  1. Implement a permanent state or flag within the vault's storage to irrevocably mark the vault as closed after emergencyClose is called. This flag should prevent any further state-altering operations.

  2. Modify the emergencyPause and emergencyResume functions to check for this permanent closure flag and revert if the vault has been emergency closed.

Updates

Lead Judging Commences

hans Auditor
almost 2 years ago
hans Auditor
almost 2 years ago
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.