Summary
During the withdrawal flow index tokens are coverted to collateral token increasing the collateral token balance but this will incorrectly lead to users taking in other users profit/tokens.
Vulnerability Details
When a user withdraws when a position is open 2x, 3x trade
All set up the Next action
Keeper call Run action
This will partially or fully close the active position and retrieve the token amount swapped in from the trade.
function afterOrderExecution(
bytes32 requestKey,
bytes32 positionKey,
IGmxProxy.OrderResultData memory orderResultData,
MarketPrices memory prices
) external nonReentrant {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
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);
}
@audit>>> } else if (orderResultData.orderType == Order.OrderType.MarketDecrease) {
uint256 sizeInUsd = vaultReader.getPositionSizeInUsd(curPositionKey);
if (sizeInUsd == 0) {
delete curPositionKey;
}
@audit>>> if (flow == FLOW.WITHDRAW) {
@audit>>> nextAction.selector = NextActionSelector.FINALIZE;
@audit>>> uint256 prevCollateralBalance = collateralToken.balanceOf(address(this)) - orderResultData.outputAmount;
@audit>>> nextAction.data = abi.encode(prevCollateralBalance, sizeInUsd == 0, false);
} else {
_updateState(true, false);
}
5 . we saved the Collateral balance before this call. to be used during finalize action
But
6 . Finalize will increase the collateral balance if we have enough index tokens in the system
* @notice `flow` is not completed in one tx. So in that case, set up the next action data
* and run the `nextAction`.
* @param prices GMX price data of market tokens from GMX API call
* @param metadata swap tx data. generated by off-chain script
*/
function runNextAction(MarketPrices memory prices, bytes[] memory metadata) external nonReentrant gmxLock {
_onlyKeeper();
Action memory _nextAction = nextAction;
delete nextAction;
if (_nextAction.selector == NextActionSelector.INCREASE_ACTION) {
(bool _isLong) = abi.decode(_nextAction.data, (bool));
if (_isLongOneLeverage(_isLong)) {
_runSwap(metadata, true, prices);
} else {
if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
(uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
_createIncreasePosition(_isLong, acceptablePrice, prices);
}
} else if (_nextAction.selector == NextActionSelector.WITHDRAW_ACTION) {
if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
uint256 depositId = flowData;
_withdraw(depositId, metadata[0], prices);
} else if (_nextAction.selector == NextActionSelector.SWAP_ACTION) {
(, bool isCollateralToIndex) = abi.decode(
_nextAction.data,
(uint256, bool)
);
_runSwap(metadata, isCollateralToIndex, prices);
} else if (_nextAction.selector == NextActionSelector.SETTLE_ACTION) {
_settle();
@audit>>> } else if (_nextAction.selector == NextActionSelector.FINALIZE) {
@audit>>> if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
@audit>>> _doDexSwap(data, false);
}
@audit>>> _finalize(_nextAction.data);
7 . Calling the dex before we finally process the withdrawal of users will increment the balance of the collateral. index => collateral token
* @dev Executes a DEX swap using Paraswap.
* @param data Swap transaction data.
* @param isCollateralToIndex Direction of swap. If true, swap `collateralToken` to `indexToken`.
* @return outputAmount The amount of output tokens received from the swap.
*/
function _doDexSwap(bytes memory data, bool isCollateralToIndex) internal returns (uint256 outputAmount) {
(address to, uint256 amount, bytes memory callData) = abi.decode(data, (address, uint256, bytes));
IERC20 inputToken;
IERC20 outputToken;
if (isCollateralToIndex) {
inputToken = collateralToken;
outputToken = IERC20(indexToken);
@audit>> } else {
inputToken = IERC20(indexToken);
@audit>> outputToken = collateralToken;
}
@audit>> uint256 balBefore = outputToken.balanceOf(address(this));
@audit>> ParaSwapUtils.swap(to, callData);
@audit>> outputAmount = IERC20(outputToken).balanceOf(address(this)) - balBefore;
emit DexSwap(address(inputToken), amount, address(outputToken), outputAmount, isCollateralToIndex);
}
8 . After runACtion as done this swap it calls finalize
if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
@audit>>> _finalize(_nextAction.data);
9 . Finalize will take the difference between the present collateral balance after adding more tokens from the index minus the balance we saved before calling finalize action and send all tokens to the user
function _finalize(bytes memory data) internal {
if (flow == FLOW.WITHDRAW) {
@audit >> (uint256 prevCollateralBalance, bool positionClosed, bool refundFee) = abi.decode(data, (uint256, bool, bool));
@audit >> uint256 withdrawn = collateralToken.balanceOf(address(this)) - prevCollateralBalance;
@audit >> _handleReturn(withdrawn, positionClosed, refundFee);
} else {
delete swapProgressData;
delete flowData;
delete flow;
}
}
10 . This order is processed and users are sent the inflated amount, withdrawn amount sent in has been corrupted/inflated
* @notice this function is an end of withdrawal flow.
* @dev should update all necessary global state variables
*
* @param withdrawn amount of token withdrawn from the position
* @param positionClosed true when position is closed completely by withdrawing all funds, or false
*/
function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
uint256 shares = depositInfo[depositId].shares;
uint256 amount;
if (positionClosed) {
amount = collateralToken.balanceOf(address(this)) * shares / totalShares;
@audit>>> } else {
@audit>>> uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
@audit>>> amount = withdrawn + balanceBeforeWithdrawal * shares / totalShares;
}
if (amount > 0) {
_transferToken(depositId, amount);
}
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId);
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}
delete swapProgressData;
delete flowData;
delete flow;
}
Impact
Users will get more than they should when ever index token in the contract at the time of the withdrawal call is greater than ONEUSD.
Tools Used
Manual review
Recommendations
Call index token swap Within finalize action when the Amount that was swapped to be withdrawn as been correctly calculated, instead of sending all index tokens in the contract to the user withdrawing.
