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

Starting an emergency flow during the 2-step deposit process will result in the complete loss of funds for the depositor

cosine

Summary

If during the 2-step process of depositing, the emergencyPause function is called, the depositor looses all deposited funds and does not receive shares for it.

Vulnerability Details

When depositing into the strategy vault, a 2-step process begins. First, a request to deposit is sent to the GMX router. Then a GMX keeper bot validates the request and if it was successful sends back the share tokens. A Steadefi keeper bot reacts on this transaction from GMX and gives the user the rightful amount of share tokens.

During these process, the status variable of the system is updated.

Inside the deposit function, the current status of the system is set to deposit:

function deposit(
GMXTypes.Store storage self,
GMXTypes.DepositParams memory dp,
bool isNative
) external {
...
self.status = GMXTypes.Status.Deposit;
...
}

If the deposit was successful, the keeper calls the processDeposit function which calls the beforeProcessDepositChecks function, mints the share tokens to the user and updates the status to Open:

function processDeposit(
GMXTypes.Store storage self
) external {
GMXChecks.beforeProcessDepositChecks(self);
​
try GMXProcessDeposit.processDeposit(self) {
self.vault.mint(self.depositCache.user, self.depositCache.sharesToUser);
​
self.status = GMXTypes.Status.Open;
​
...
}

And the beforeProcessDepositChecks function called at the beginning reverts if the current status is not Deposit:

function beforeProcessDepositChecks(
GMXTypes.Store storage self
) external view {
if (self.status != GMXTypes.Status.Deposit)
revert Errors.NotAllowedInCurrentVaultStatus();
}

So as we can see, the current status must be Deposit so that the user receives shares after a successful deposit, otherwise the funds of the user are gone.

Now if during this process of the user calling deposit and the keeper reacting to it and calling processDeposit, the owner calls emergencyPause, the status will be updated to Paused and therefore processDeposit reverts:

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

Therefore, all funds of the depositing user are gone and instead handled as yield and shared between all other users who deposited earlier.

The flow would look like that:

  • User calls Deposit [User's funds are gone but did not receive shares yet] (Status = Deposit)

  • Keeper calls emergencyPause (Status = Paused)

  • Keeper calls processDeposit

  • processDeposit fails as the check if self.status != GMXTypes.Status.Deposit reverts because the system is in (Status = Paused, Closed, Resume, or Open, depending on the Emergency Flow timing)

  • User does never receive shares and therefore loses all the funds deposited

Impact

Depositor loses all funds deposited.

Tools Used

Manual Review

Recommendations

If there is a running deposit flow when calling emergencyPause handle it properly.

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.