Summary
Users can withdraw all his margins and back his position with only positive PNL to avoid paying fees during liquidation
Vulnerability Details
When a user deposits some margin, opens a position and that position get positive unrealised PNL, he can withdraw all his deposited margin. At this point the position will be backed entirely by the unrealised PNL. The problem here is that if the position gets to a liquidable state, no fees will be deducted to the user because he does not have any margin, so no fees will be sent to receivers.
Let's take a look at how it works.
function withdrawMargin(uint128 tradingAccountId, address collateralType, uint256 amount) external {
MarginCollateralConfiguration.Data storage marginCollateralConfiguration =
MarginCollateralConfiguration.load(collateralType);
TradingAccount.Data storage tradingAccount =
TradingAccount.loadExistingAccountAndVerifySender(tradingAccountId);
UD60x18 amountX18 = marginCollateralConfiguration.convertTokenAmountToUd60x18(amount);
_requireAmountNotZero(amountX18);
_requireEnoughMarginCollateral(tradingAccount, collateralType, amountX18);
tradingAccount.withdraw(collateralType, amountX18);
(UD60x18 requiredInitialMarginUsdX18,, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
tradingAccount.validateMarginRequirement(requiredInitialMarginUsdX18, marginBalanceUsdX18, UD60x18_ZERO);
IERC20(collateralType).safeTransfer(msg.sender, amount);
emit LogWithdrawMargin(msg.sender, tradingAccountId, collateralType, amount);
}
When a user wants to withdraw margin that he deposited this code is executed, in it we can extract the following checks:
Verify that the caller is the owner of the position
Enforce that the amount to withdraw is greater than 0
Enforce that the amount to withdraw is less than or equal to the margin deposited
With the margin removed from the account, ensure that the margin + PnL can back the position
Transfer the requested margin to the user
As we can see, if a user has enough PnL can fully back his position and withdraw all his margin. With this state, if the position was liquidated it would execute the following:
function liquidateAccounts(uint128[] calldata accountsIds) external {
...
for (uint256 i; i < accountsIds.length; i++) {
...
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
pnlUsdX18: requiredMaintenanceMarginUsdX18,
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});
...
}
}
function deductAccountMargin(
Data storage self,
FeeRecipients.Data memory feeRecipients,
UD60x18 pnlUsdX18,
UD60x18 settlementFeeUsdX18,
UD60x18 orderFeeUsdX18
)
internal
returns (UD60x18 marginDeductedUsdX18)
{
DeductAccountMarginContext memory ctx;
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
uint256 cachedCollateralLiquidationPriorityLength = globalConfiguration.collateralLiquidationPriority.length();
for (uint256 i; i < cachedCollateralLiquidationPriorityLength; i++) {
address collateralType = globalConfiguration.collateralLiquidationPriority.at(i);
MarginCollateralConfiguration.Data storage marginCollateralConfiguration =
MarginCollateralConfiguration.load(collateralType);
ctx.marginCollateralBalanceX18 = getMarginCollateralBalance(self, collateralType);
if (ctx.marginCollateralBalanceX18.isZero()) continue;
...
}
marginDeductedUsdX18 =
ctx.settlementFeeDeductedUsdX18.add(ctx.orderFeeDeductedUsdX18).add(ctx.pnlDeductedUsdX18);
}
As we can see, the liquidation function will try to deduct the liquidation fee and the required maintenance margin but when the deductAccountMargin
will be called, since the position will not have any margin, the loop will not execute any transfer and the fee receiver will not receive anything.
Impact
This situation can be dangerous for the protocol because keepers should get fees to pay for the service and also the protocol should get at least the maintenance margin in order to have solvency.
Tools Used
Manual review
Recommendations
There is not a trivial solution, but I would recommend to only allow to withdraw the margin only if it keeps a minimum to cover some fees.