Summary
} else if (orderResultData.orderType == Order.OrderType.MarketDecrease) {
uint256 sizeInUsd = vaultReader.getPositionSizeInUsd(curPositionKey);
if (sizeInUsd == 0) {
delete curPositionKey;
}
function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) {
delete curPositionKey;
}
The curPositionKey
is only initialized when closing all positions or during liquidation, so using this, an attacker can withdraw more tokens.
Steps to Reproduce
Since the test code does not work, here’s an explanation of the attack sequence.
The attacker opens a new small position.
The attacker waits until another user creates a position.
The curPositionKey
variable is set when opening a new position and executing the afterOrderExecution
function.
The attacker calls the withdrawal function. The following conditions are required:
function withdraw(address recipient, uint256 depositId) public payable nonReentrant {
_noneFlow();
flow = FLOW.WITHDRAW;
flowData = depositId;
...
if (curPositionKey != bytes32(0)) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
_settle();
}
Since curPositionKey
is already set, the WITHDRAW_ACTION
will be executed.
} else if (_nextAction.selector == NextActionSelector.WITHDRAW_ACTION) {
...
uint256 depositId = flowData;
_withdraw(depositId, metadata[0], prices);
}
function _withdraw(uint256 depositId, bytes memory metadata, MarketPrices memory prices) internal {
uint256 shares = depositInfo[depositId].shares;
...
if (positionIsClosed) {
_handleReturn(0, true, false);
} else if (_isLongOneLeverage(beenLong)) {
uint256 swapAmount = IERC20(indexToken).balanceOf(address(this)) * shares / totalShares;
nextAction.selector = NextActionSelector.SWAP_ACTION;
nextAction.data = abi.encode(swapAmount, false);
} else if (curPositionKey == bytes32(0)) {
_handleReturn(0, true, false);
} else {
IVaultReader.PositionData memory positionData = vaultReader.getPositionInfo(curPositionKey, prices);
uint256 collateralDeltaAmount = positionData.collateralAmount * shares / totalShares;
uint256 sizeDeltaInUsd = positionData.sizeInUsd * shares / totalShares;
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, sizeDeltaInUsd, false) / prices.shortTokenPrice.max;
int256 pnl = vaultReader.getPnl(curPositionKey, prices, sizeDeltaInUsd);
if (pnl < 0) {
collateralDeltaAmount = collateralDeltaAmount - feeAmount - uint256(-pnl) / prices.shortTokenPrice.max;
} else {
collateralDeltaAmount = collateralDeltaAmount - feeAmount;
}
uint256 acceptablePrice = abi.decode(metadata, (uint256));
_createDecreasePosition(collateralDeltaAmount, sizeDeltaInUsd, beenLong, acceptablePrice, prices);
}
}
If positionIsClosed
is false and _isLongOneLeverage(beenLong)
is false, the _createDecreasePosition
function will be executed. The issue here is that curPosition
belongs to another user's position, and that position is always larger than the attacker's position. By exploiting this, the attacker can close a larger position and make a profit.
References