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

Reborrowing of unpaid debt can increase vault leverage

hash

Vulnerability Details

When redepositing liquidity in processWithdrawFailure, the repay amounts are again borrowed even though the previous debt's have not been repaid. This will increase the leverage of the vault.

function processWithdrawFailure(
GMXTypes.Store storage self,
uint256 slippage,
uint256 executionFee
) external {
GMXChecks.beforeProcessAfterWithdrawFailureChecks(self);
​
// @audit the borrowed assets have not been repaid if GMXProcessWithdraw.processWithdraw() has reverted
// Re-borrow assets based on the repaid amount
GMXManager.borrow(
self,
self.withdrawCache.repayParams.repayTokenAAmt,
self.withdrawCache.repayParams.repayTokenBAmt
);
​
​
// Re-add liquidity using all available tokenA/B in vault
GMXTypes.AddLiquidityParams memory _alp;
​
​
_alp.tokenAAmt = self.tokenA.balanceOf(address(this));
_alp.tokenBAmt = self.tokenB.balanceOf(address(this));

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

The previous debt is unpaid if GMXProcessWithdraw.processWithdraw() reverts

function processWithdraw(
GMXTypes.Store storage self
) external {
..... more debt
​
// Repay debt
GMXManager.repay(
self,
self.withdrawCache.repayParams.repayTokenAAmt,
self.withdrawCache.repayParams.repayTokenBAmt
);
​
..... more code
​
GMXChecks.afterWithdrawChecks(self);
}

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

GMXProcessWithdraw.processWithdraw() can be forcefully reverted by a user by providing a very high minWithdrawTokenAmt

function afterWithdrawChecks(
GMXTypes.Store storage self
) external view {
....... more code
​
if (
self.withdrawCache.tokensToUser <
self.withdrawCache.withdrawParams.minWithdrawTokenAmt
) revert Errors.InsufficientAssetsReceived();
}

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXChecks.sol#L203-L228

Impact

The leverage of the vault can be forcefully increased by any depositor.

Recommendations

Use the funds returned from GMX instead of reborrow

diff --git a/contracts/strategy/gmx/GMXWithdraw.sol b/contracts/strategy/gmx/GMXWithdraw.sol
index b957df1..e4f7b84 100644
--- a/contracts/strategy/gmx/GMXWithdraw.sol
+++ b/contracts/strategy/gmx/GMXWithdraw.sol
@@ -235,13 +235,6 @@ library GMXWithdraw {
) external {
GMXChecks.beforeProcessAfterWithdrawFailureChecks(self);
- // Re-borrow assets based on the repaid amount
- GMXManager.borrow(
- self,
- self.withdrawCache.repayParams.repayTokenAAmt,
- self.withdrawCache.repayParams.repayTokenBAmt
- );
-
// Re-add liquidity using all available tokenA/B in vault
GMXTypes.AddLiquidityParams memory _alp;
Updates

Lead Judging Commences

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

Wrong logic in processWithdrawFailure

Impact: High Likelihood: High Overlending is caused due to unnecessary re-borrow on processWithdrawFailure. Assumption that the repayment had gone because it was in try-catch is incorrect.

Support

FAQs

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