H-09. The `afterWithdrawChecks` applies only if user wants to withdraw in tokenA/B

Submitted by Drynooo, pontifex. Selected submission by: pontifex.

Relevant GitHub Links

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXProcessWithdraw.sol#L104

Summary

The afterWithdrawChecks check is very important to be sure that important health parameters are in the proper ranges. But the check is inside brackets of the if user wants to withdraw in tokenA/B statement. So if the user wants to withdraw LP-token the check is not provided. This can cause unexpected financial losses.

Vulnerability Details

The afterWithdrawChecks check is placed inside the brackets of the if-statement of the GMXProcessWithdraw.processWithdraw function. This statement checks if user wants to withdraw in tokenA/B. In other cases the afterWithdrawChecks check is not provided but should.

plain text
69    // Else if user wants to withdraw in LP token, the tokensToUser is already previously
70    // set in GMXWithdraw.withdraw()
71    if (
72      self.withdrawCache.withdrawParams.token == address(self.tokenA) ||
73      self.withdrawCache.withdrawParams.token == address(self.tokenB)
74    ) {

104      GMXChecks.afterWithdrawChecks(self);
105    }
106  }

Impact

The issue can cause unexpected financial losses.

Tools used

Manual Review

Recommendations

I suppose that the check should be placed after the if statement brackets.