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

Fee is minted only after the first withdrawal in a block

hash

Vulnerability Details

When withdrawing, the pending fee is minted only after the withdrawal asset amounts are calculated. This allows the first withdrawal to not incur any fees meanwhile the remaining user's are burdened with additional fees.

function withdraw(
GMXTypes.Store storage self,
GMXTypes.WithdrawParams memory wp
) external {
..... more code
​
// @audit the withdrawal lp amount calculated doesn't include the pending fees
​
_wc.shareRatio = wp.shareAmt
* SAFE_MULTIPLIER
/ IERC20(address(self.vault)).totalSupply();
_wc.lpAmt = _wc.shareRatio
* GMXReader.lpAmt(self)
/ SAFE_MULTIPLIER;
_wc.withdrawValue = _wc.lpAmt
* self.gmxOracle.getLpTokenValue(
address(self.lpToken),
address(self.tokenA),
address(self.tokenA),
address(self.tokenB),
false,
false
)
/ SAFE_MULTIPLIER;
​
​
...... more code
​
// @audit the pending fee is minted here. The fee minted calculation will also include the to be withdrawed shares which will cause the remaining user's to pay higher fees
self.vault.mintFee();

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXWithdraw.sol#L41-L101

Impact

First withdrawals will avoid fees while other user's will incur more fees than actual.

Recommendations

Mint fees before calculating the withdrawal lp amount.

diff --git a/contracts/strategy/gmx/GMXWithdraw.sol b/contracts/strategy/gmx/GMXWithdraw.sol
index b957df1..92c78ad 100644
--- a/contracts/strategy/gmx/GMXWithdraw.sol
+++ b/contracts/strategy/gmx/GMXWithdraw.sol
@@ -42,6 +42,7 @@ library GMXWithdraw {
GMXTypes.Store storage self,
GMXTypes.WithdrawParams memory wp
) external {
+ self.vault.mintFee();
// Sweep any tokenA/B in vault to the temporary trove for future compouding and to prevent
// it from being considered as part of withdrawers assets
if (self.tokenA.balanceOf(address(this)) > 0) {
@@ -98,8 +99,6 @@ library GMXWithdraw {
self.status = GMXTypes.Status.Withdraw;
- self.vault.mintFee();
-
GMXTypes.RemoveLiquidityParams memory _rlp;
// If user wants to withdraw LP tokens, only remove liquidity of
​
Updates

Lead Judging Commences

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

Fee not accounted during withdrawal

Impact: High Likelihood: High User share amount is calculated before minting fee and the remaining users will need to more fee than reasonable.

Support

FAQs

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