Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Loss of withdrawal fee refund when withdraw from GMX position

jokerstudio

Summary

The Gamma protocol will refund the excess withdrawal execution fee to the user when the flow is finalized. When user withdraw their deposit on any two or more leverage vault, they need to paid upfront the execution fee, and then wait for the vault and GMX interaction to finalize the flow, in the end the user will receive the refund of the excess execution fee.

The issue occurs when the PerpetualVault::afterOrderExecution set the incorrect value that causes user will not receive the refund of the excess execution fee that shown in the vulnerability details.

Vulnerability Details

The following steps shows the vulnerability flow:

  1. The User call PerpetualVault::withdraw to withdraw the deposit and paid the execution fee upfront.

  2. The Keeper call PerpetualVault::runNextAction to run the WITHDRAW_ACTION.
    At this step, if the GMX position is open, then create a GMX order to decrease the position size.

  3. The callback function PerpetualVault::afterOrderExecution will be called with the Order.OrderType.MarketDecrease
    At this step, the root cause of this issue occur when the encoded refundFee part of nextAction.data will be set to false

  4. The Keeper call PerpetualVault::runNextAction to run the FINALIZE by a given nextAction.data set before.

Finally, the _handleReturn function will be called with the refundFee = false and the user will not receive the refund of the excess execution fee.

function afterOrderExecution(
bytes32 requestKey,
bytes32 positionKey,
IGmxProxy.OrderResultData memory orderResultData,
MarketPrices memory prices
) external nonReentrant {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
// MarketPrices memory marketPrices = gmxProxy.getMarketPrices(market);
_gmxLock = false;
// If the current action is `settle`
if (orderResultData.isSettle) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
emit GmxPositionCallbackCalled(requestKey, true);
return;
}
if (orderResultData.orderType == Order.OrderType.MarketIncrease) {
curPositionKey = positionKey;
if (flow == FLOW.DEPOSIT) {
uint256 amount = depositInfo[counter].amount;
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, orderResultData.sizeDeltaUsd, false) / prices.shortTokenPrice.min;
uint256 prevSizeInTokens = flowData;
int256 priceImpact = vaultReader.getPriceImpactInCollateral(curPositionKey, orderResultData.sizeDeltaUsd, prevSizeInTokens, prices);
uint256 increased;
if (priceImpact > 0) {
increased = amount - feeAmount - uint256(priceImpact) - 1;
} else {
increased = amount - feeAmount + uint256(-priceImpact) - 1;
}
_mint(counter, increased, false, prices);
nextAction.selector = NextActionSelector.FINALIZE;
} else {
_updateState(false, orderResultData.isLong);
}
} else if (orderResultData.orderType == Order.OrderType.MarketDecrease) {
uint256 sizeInUsd = vaultReader.getPositionSizeInUsd(curPositionKey);
if (sizeInUsd == 0) {
delete curPositionKey;
}
if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.FINALIZE;
uint256 prevCollateralBalance = collateralToken.balanceOf(address(this)) - orderResultData.outputAmount;
@> nextAction.data = abi.encode(prevCollateralBalance, sizeInUsd == 0, false);
...
}
function runNextAction(MarketPrices memory prices, bytes[] memory metadata) external nonReentrant gmxLock {
_onlyKeeper();
Action memory _nextAction = nextAction;
delete nextAction;
...
_runSwap(metadata, isCollateralToIndex, prices);
} else if (_nextAction.selector == NextActionSelector.SETTLE_ACTION) {
_settle();
} else if (_nextAction.selector == NextActionSelector.FINALIZE) {
if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
@> _finalize(_nextAction.data);
} else if (positionIsClosed == false && _isFundIdle()) {
flow = FLOW.COMPOUND;
if (_isLongOneLeverage(beenLong)) {
_runSwap(metadata, true, prices);
} else {
(uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
_createIncreasePosition(beenLong, acceptablePrice, prices);
}
} else {
revert Error.InvalidCall();
}
}
function _finalize(bytes memory data) internal {
if (flow == FLOW.WITHDRAW) {
(uint256 prevCollateralBalance, bool positionClosed, bool refundFee) = abi.decode(data, (uint256, bool, bool));
uint256 withdrawn = collateralToken.balanceOf(address(this)) - prevCollateralBalance;
@> _handleReturn(withdrawn, positionClosed, refundFee);
} else {
delete swapProgressData;
delete flowData;
delete flow;
}
}

Impact

Users do not receive refunds when withdrawing their deposits on any two or more leverage vaults when it has a GMX position open. Especially, the PerpetualVault::_payExecutionFee function allows users to send an arbitrary amount exceeding the estimated execution fee without restriction. Consequently, users will lose all fees without getting any refund, and the execution fee becomes stuck in the GmxProxy contract.

Please note that although the protocol has the GmxProxy::withdrawEth function that allows owner withdraw ethers in some accident. However, this function should not be called in the normal flow.

PoC

  1. Copy the getParaSwapData_Index_To_Collateral function to the test/mock/MockData.sol for support swap index token back to collateral token for withdraw flow.

  2. Copy the following test case to the test/PerpetualVault.t.sol file

  3. Run test with forge test --mt test_Loss_Of_GasFeeRefund_When_Withdraw_FromGmxPosition --rpc-url arbitrum

function getParaSwapData_Index_To_Collateral(address receiver) external pure returns (bytes memory) {
bytes memory rev = abi.encodePacked(receiver);
bytes memory original = hex'000000000000000000000000def171fe48cf0115b1d80b88dc8eab59176fee57000000000000000000000000000000000000000000000000287a7d29bb1d81ed000000000000000000000000000000000000000000000000000000000000006000000000000000000000000000000000000000000000000000000000000007c446c67b6d000000000000000000000000000000000000000000000000000000000000002000000000000000000000000082af49447d8a07e3bd95bd0d56f35241523fbab1000000000000000000000000000000000000000000000000287a7d29bb1d81ed00000000000000000000000000000000000000000000000000000002440be40000000000000000000000000000000000000000000000000000000002540be400000000000000000000000000919c94b69950449cea621fd6cc0cb538de79d0dd000000000000000000000000000000000000000000000000000000000000016000000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000078000000000000000000000000000000000000000000000000000000000679cf4ce161d4b0c6e2e4ca381c524a0776f557100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000002710000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000020000000000000000000000000af88d065e77c8cc2239327c5edb3a432268e5831000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000002a000000000000000000000000058a5f0b73969800faff8556cd2187e3fce71a6cb0000000000000000000000000000000000000000000000000000000000001f40000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000070000000000000000000000001f721e2e82f6676fce4ea07a5958cf098d339e18000000000000000000000000000000000000000000000000000000000000271000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000e0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000067a62e220000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002882af49447d8a07e3bd95bd0d56f35241523fbab1af88d065e77c8cc2239327c5edb3a432268e5831000000000000000000000000000000000000000000000000000000000000000000000000369a2fdb910d432f0a07381a5e3d27572c87671300000000000000000000000000000000000000000000000000000000000007d0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000030000000000000000000000001b81d678ffb9c0263b24a97847620c99d213eb14000000000000000000000000000000000000000000000000000000000000271000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000c0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000067a62e22000000000000000000000000000000000000000000000000000000000000002b82af49447d8a07e3bd95bd0d56f35241523fbab1000064af88d065e77c8cc2239327c5edb3a432268e5831000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000';
​
bytes memory result = original;
​
assembly {
let originalPtr := add(result, 0x20)
let replacementPtr := add(rev, 0x20)
​
for {let i := 0} lt(i, mload(rev)) { i := add(i, 0x20) } {
mstore(add(originalPtr, add(304, i)), mload(add(replacementPtr, i)))
}
}
require(original.length == result.length, "fail");
return result;
}
function test_Loss_Of_GasFeeRefund_When_Withdraw_FromGmxPosition() external {
address keeper = PerpetualVault(vault2x).keeper();
address gmxProxy = address(PerpetualVault(vault2x).gmxProxy());
uint256 gmxProxyBalBefore = gmxProxy.balance;
address alice = makeAddr("alice");
depositFixtureInto2x(alice, 1e10);
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3390000000000000);
vm.prank(keeper);
PerpetualVault(vault2x).run(true, true, prices, data);
PerpetualVault.FLOW flow = PerpetualVault(vault2x).flow();
assertEq(uint8(flow), 2);
​
GmxOrderExecuted2x(true);
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices, new bytes[](0));
​
uint256[] memory depositIds = PerpetualVault(vault2x).getUserDeposits(alice);
uint256 lockTime = 1;
PerpetualVault(vault2x).setLockTime(lockTime);
vm.warp(block.timestamp + lockTime + 1);
​
deal(alice, 1 ether);
vm.prank(alice);
PerpetualVault(vault2x).withdraw{value: alice.balance}(alice, depositIds[0]); // alice mistakenly sent all balance of ether as execution fee
GmxOrderExecuted2x(true); // execute GMX settle order
​
bytes memory paraSwapData = mockData.getParaSwapData_Index_To_Collateral(vault2x);
bytes[] memory swapData = new bytes[](1);
swapData[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices, swapData); // run WITHDRAW_ACTION for WITHDRAW flow
​
GmxOrderExecuted2x(true); // execute GMX MarketDecrease order
​
vm.prank(keeper);
PerpetualVault(vault2x).runNextAction(prices, swapData); // run FINALIZE for WITHDRAW flow
​
// withdrawal flow is finalized
flow = PerpetualVault(vault2x).flow();
assertEq(uint8(flow), 0);
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 0);
​
assertEq(alice.balance, 0); // alice loss all ether without getting any refund!
assertTrue(gmxProxy.balance > gmxProxyBalBefore); //alice`s execution fee becomes stuck in the GmxProxy!
}

Tools Used

Manual Review

Recommendations

Within the PerpetualVault::afterOrderExecution function, change the part of encoded refundFee to true.

function afterOrderExecution(
bytes32 requestKey,
bytes32 positionKey,
IGmxProxy.OrderResultData memory orderResultData,
MarketPrices memory prices
) external nonReentrant {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
// MarketPrices memory marketPrices = gmxProxy.getMarketPrices(market);
_gmxLock = false;
// If the current action is `settle`
if (orderResultData.isSettle) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
emit GmxPositionCallbackCalled(requestKey, true);
return;
}
if (orderResultData.orderType == Order.OrderType.MarketIncrease) {
curPositionKey = positionKey;
if (flow == FLOW.DEPOSIT) {
uint256 amount = depositInfo[counter].amount;
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, orderResultData.sizeDeltaUsd, false) / prices.shortTokenPrice.min;
uint256 prevSizeInTokens = flowData;
int256 priceImpact = vaultReader.getPriceImpactInCollateral(curPositionKey, orderResultData.sizeDeltaUsd, prevSizeInTokens, prices);
uint256 increased;
if (priceImpact > 0) {
increased = amount - feeAmount - uint256(priceImpact) - 1;
} else {
increased = amount - feeAmount + uint256(-priceImpact) - 1;
}
_mint(counter, increased, false, prices);
nextAction.selector = NextActionSelector.FINALIZE;
} else {
_updateState(false, orderResultData.isLong);
}
} else if (orderResultData.orderType == Order.OrderType.MarketDecrease) {
uint256 sizeInUsd = vaultReader.getPositionSizeInUsd(curPositionKey);
if (sizeInUsd == 0) {
delete curPositionKey;
}
if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.FINALIZE;
uint256 prevCollateralBalance = collateralToken.balanceOf(address(this)) - orderResultData.outputAmount;
- nextAction.data = abi.encode(prevCollateralBalance, sizeInUsd == 0, false);
+ nextAction.data = abi.encode(prevCollateralBalance, sizeInUsd == 0, true);
...
}
Updates

Lead Judging Commences

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

invalid_no_refund

According the sponsor, here is the design choice: - when there's gmx interaction, no refund. - when there's no gmx interaction, refund fee. The remaining fees are used for gas cost of the keepers.

Support

FAQs

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