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

processWithdrawFailure borrows twice from the lending vault, increases debt without collateral and can lead to a stuck vault

cosine

Summary

When a user calls the withdraw function, liquidity is removed and the repay amount which the lending vault should receive is calculated. After that, a keeper will call processWithdraw, which will try to repay the lending vault and withdraw funds and if this doesn’t work out will instead update the status of the vault to Withdraw_Failed and call the processWithdrawFailure function. This function borrows the repaid funds again, but they were never repaid as the processWithdraw function reverted. Therefore, the funds are either borrowed twice, which will increase the debt, without further collateral, or if there are not enough funds to borrow in the lending vault the vault is stuck inside the Withdraw_Failed status. Depending on the amount which needs to be borrowed twice the system is forced into emergency action.

Vulnerability Details

The current status of the vault is Open and a user calls the withdraw function. This function calculates the amount to repay to the lending vault and updates the vault's status to Withdraw:

function withdraw(
GMXTypes.Store storage self,
GMXTypes.WithdrawParams memory wp
) external {
...
(
uint256 _repayTokenAAmt,
uint256 _repayTokenBAmt
) = GMXManager.calcRepay(self, _wc.shareRatio);
​
_wc.repayParams.repayTokenAAmt = _repayTokenAAmt;
_wc.repayParams.repayTokenBAmt = _repayTokenBAmt;
​
self.withdrawCache = _wc;
​
GMXChecks.beforeWithdrawChecks(self);
​
self.status = GMXTypes.Status.Withdraw;
...
}

After that, a keeper bot calls the process Withdraw function. This function calls the processWithdraw function inside the GMXProcessWithdraw, which will try to repay the lending vault and do some other things and if they do not work out the catch block will update the status of the vault to Withdraw_failed:

function processWithdraw(
GMXTypes.Store storage self
) external {
...
try GMXProcessWithdraw.processWithdraw(self) {
...
} catch (bytes memory reason) {
self.status = GMXTypes.Status.Withdraw_Failed;
​
emit WithdrawFailed(reason);
}
}

Here we can see processWithdraw function inside the GMXProcessWithdraw contract, which tries to repay the lending vault and therefore if it reverts and the call lands in the catch block, the lending vault was never repaid:

function processWithdraw(
GMXTypes.Store storage self
) external {
(
bool _swapNeeded,
address _tokenFrom,
address _tokenTo,
uint256 _tokenToAmt
) = GMXManager.calcSwapForRepay(self, self.withdrawCache.repayParams);
​
if (_swapNeeded) {
ISwap.SwapParams memory _sp;
​
_sp.tokenIn = _tokenFrom;
_sp.tokenOut = _tokenTo;
_sp.amountIn = IERC20(_tokenFrom).balanceOf(address(this));
_sp.amountOut = _tokenToAmt;
_sp.slippage = self.minSlippage;
_sp.deadline = block.timestamp;
​
GMXManager.swapTokensForExactTokens(self, _sp);
}
​
// Repay debt
GMXManager.repay(
self,
self.withdrawCache.repayParams.repayTokenAAmt,
self.withdrawCache.repayParams.repayTokenBAmt
);
...
}

Now if the vault's status is Withdraw_failed a keeper calls processWithdrawFailure which borrows the calculated repaid amount (which was never repaid as the processWithdraw function reverted) again from the lending vault:

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
);
...
}

So if a keeper calls processWithdrawFailure it will either borrow the funds twice from the lending vault and therefore increase leverage / debt and borrow funds without any further collateral. Or even worse, if there are not enough funds left in the lending vault, the borrow call will revert and the status of the vault will not be updated to Open again. Therefore, the vault is stuck inside this status and all users experience a full DoS as long as there are not enough funds to borrow in the lending vault. Depending on the amount which needs to be borrowed twice, this could force the owners of the vault into emergency actions.

Here we can see the borrow flow and of course that it will revert if there are not enough funds in the lending vault:

function borrow(
GMXTypes.Store storage self,
uint256 borrowTokenAAmt,
uint256 borrowTokenBAmt
) public {
if (borrowTokenAAmt > 0) {
self.tokenALendingVault.borrow(borrowTokenAAmt);
}
if (borrowTokenBAmt > 0) {
self.tokenBLendingVault.borrow(borrowTokenBAmt);
}
}
function borrow(uint256 borrowAmt) external nonReentrant whenNotPaused onlyBorrower {
if (borrowAmt == 0) revert Errors.InsufficientBorrowAmount();
if (borrowAmt > totalAvailableAsset()) revert Errors.InsufficientLendingLiquidity();
...
}

Impact

Funds are borrowed twice from the lending vault without further collateral and debt is increased, if there are enough funds in the lending vault. If there are not enough funds in the lending vault, all users experience a full DoS as long as there are not enough funds to borrow. Depending on the amount which needs to be borrowed twice, this could force the owners of the vault into emergency actions.

Tools Used

Manual Review

Recommendations

Remove the borrow call inside processWithdrawFailure as the funds were never repaid in this state.

Updates

Lead Judging Commences

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