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

emergencyPause may cause damage to the user in the middle of an operation.

drynooo

Summary

The emergencyPause function can be executed in any state and change the state to Paused. which may cause the running state to not continue.

Vulnerability Details

Take deposit for example, if a user is currently depositing and the current state is Deposit, the user has already paid for the asset but has not received the corresponding share. if the emergencyPause function is executed at this time and the state is changed to Paused, then the afterDepositExecution function is used to change the state to Paused. function, the processDeposit cannot continue and the user loses the funds.

function afterDepositExecution(
bytes32 depositKey,
IDeposit.Props memory /* depositProps */,
IEvent.Props memory /* eventData */
) external onlyController {
GMXTypes.Store memory _store = vault.store();
​
if (
_store.status == GMXTypes.Status.Deposit &&
_store.depositCache.depositKey == depositKey
) {
vault.processDeposit();

Impact

Users may lose funds.

Tools Used

manual

Recommendations

I think a better approach would be to normally execute functions that can be executed in the callback even in Paused state, such as processDeposit. this would avoid user loss.
Or even if it is not modified, this risk should still be made clear to everyone.

Updates

Lead Judging Commences

hans Lead Judge over 1 year 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.