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

incorrect logic to handle deposit failure by reborrowing more .

ElHaj

Summary

  • When a successful withdrawal is executed from the GMX market but the callback fails, the status is set to withdraw_failed. Subsequently, the keeper calls the processWithdrawFailure function, which incorrectly handles this event by reborrowing again and then adding liquidity.

Vulnerability Details

  • The processWithdrawFailure function is invoked after a withdrawal failure. In this scenario, it reborrows the previous repayTokenAAmt and repayTokenBAmt from the lendingVault contract and subsequently adds liquidity again:

function processWithdrawFailure(GMXTypes.Store storage self, uint256 slippage, uint256 executionFee) 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;
​
_alp.tokenAAmt = self.tokenA.balanceOf(address(this));
_alp.tokenBAmt = self.tokenB.balanceOf(address(this));
​
// Calculate slippage
uint256 _depositValue = GMXReader.convertToUsdValue(
self, address(self.tokenA), self.tokenA.balanceOf(address(this))
) + GMXReader.convertToUsdValue(self, address(self.tokenB), self.tokenB.balanceOf(address(this)));
​
_alp.minMarketTokenAmt = GMXManager.calcMinMarketSlippageAmt(self, _depositValue, slippage);
_alp.executionFee = executionFee;
​
// Re-add liquidity with all tokenA/tokenB in vault
>> self.withdrawCache.depositKey = GMXManager.addLiquidity(self, _alp);
}
​

  • This logic is flawed because there is no need to re-borrow in the withdraw_failed status. The status being withdraw_failed indicates the contract's initial failure to repay the debt, and the withdrawn tokens are already held by the strategyVault. The withdraw_failed status is set only when the try block inside processWithdraw fails:

function processWithdraw(GMXTypes.Store storage self) external {
// the contract repay the debt in the try call :
try GMXProcessWithdraw.processWithdraw(self) {
// some code ....
} catch (bytes memory reason) {
// withdraw failed only can be set here .
self.status = GMXTypes.Status.Withdraw_Failed;
emit WithdrawFailed(reason);
}
}

  • In the try block, the processWithdraw function from GMXProcessWithdraw library is called, where the vault repays the debt:

function processWithdraw(GMXTypes.Store storage self) external {
// some code ....
​
// Repay debt
>> GMXManager.repay(
self,
self.withdrawCache.repayParams.repayTokenAAmt, // same issue
self.withdrawCache.repayParams.repayTokenBAmt
);
// some other code ..
​
}

However, the flawed logic results in the contract borrowing the same amounts again, mistakenly assuming the debt has already been repaid.

  • Another issue arises when there is insufficient funds in the lending contract for borrowing, as the system does not check the capacity beforehand. This results in repeated reverting transactions, causing the contract to remain stuck in the withdraw_failed status. Continuous calls from the keeper only result in wasted gas without any progress.

Impact

  • Increased risk of bad debt in case of strategy losses.

  • Higher interest costs for lending.

  • Potential contract getting stuck at withdraw_failed status.

Tools Used

vs code
manual review

Recommendations

  • there is no need for reborrowing . just add liquidity again with the token held by the strategyVault

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.