Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Low level call in processDepositCancellation might result in extra funds transfered to user

DarkTower

Summary

Function processDepositCancellation in GMXDeposit.sol transfers all contract ether to the Steadefi depositor whose deposit was canceled. Problem is in this line:

(bool success, ) = self.depositCache.user.call{value: address(this).balance}("");

It transfers the entire contract ether balance to one user. The design assumes that only Steadefi depositor's ether would ever be locked in the contract. However anybody can transfer ether to the GMXVault.sol from which function processDepositCancellation is called.

As the result the current design leaves a way for any steadefi depositor to claim ether funds transfered to the GMXVault.sol

Vulnerability Details

Here is the code of processDepositCancellation function:

function processDepositCancellation(
GMXTypes.Store storage self
) external {
GMXChecks.beforeProcessDepositCancellationChecks(self);
​
// Repay borrowed assets
GMXManager.repay(
self,
self.depositCache.borrowParams.borrowTokenAAmt,
self.depositCache.borrowParams.borrowTokenBAmt
);
​
// Return user's deposited asset
// If native token is being withdrawn, we convert wrapped to native
if (self.depositCache.depositParams.token == address(self.WNT)) {
self.WNT.withdraw(self.WNT.balanceOf(address(this)));
(bool success, ) = self.depositCache.user.call{value: address(this).balance}("");
require(success, "Transfer failed.");
} else {
// Transfer requested withdraw asset to user
IERC20(self.depositCache.depositParams.token).safeTransfer(
self.depositCache.user,
self.depositCache.depositParams.amt
);
}
​
self.status = GMXTypes.Status.Open;
​
emit DepositCancelled(self.depositCache.user);
}

As can be seen, it uses a low level call to transfer the entire ether balance of the contract to one user here:

(bool success, ) = self.depositCache.user.call{value: address(this).balance}("");

The processDepositCancellation is called in GMXVault.sol which has receive() external payable so it can recieve ether from anybody.

All said above opens an opportunity for the following scenario:

  1. Hacker monitors the GMXVault.sol contract to see when ether gets deposited there

  2. Once spotted, the hacker deposits to Steadefi protocol and orchestrates the cancelation of the deposit so that processDepositCancellation is called

  3. All contract's ether is transfered to the hacker

Impact

In case any ether is transfered to the contract, then such ether would be transfered to one particlar depositor on processDepositCancellation call and essentially lost for the actual owner of the ether who transfered it to the contract in the first place. A sophisticated hacker would be interested in monitoring contract balance to claim any deposited ether by orchestrating a cancelation of the deposit on Steadefi protocol.

Tools Used

Manual research.

Recommendations

Make sure that only Steadefi users' deposited ether is withdrawn when processDepositCancellation is called.

Updates

Lead Judging Commences

hans Lead Judge about 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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

Give us feedback!