Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

liquidateAccounts() lack of cancel all offchain orders

0x1982us

Summary

when liquidateAccounts()
will clear pedding MarketOrder
but don't cancel all offchain orders

Vulnerability Details

when liquidateAccounts()
we will clear pedding MarketOrder

function liquidateAccounts(uint128[] calldata accountsIds) external {
...
​
// deduct maintenance margin from the account's collateral
// settlementFee = liquidationFee
ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
//accountTotalUnrealizedPnlUsdX18
pnlUsdX18: requiredMaintenanceMarginUsdX18,
orderFeeUsdX18: UD60x18_ZERO,
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});
​
// clear pending order for account being liquidated
@> MarketOrder.load(ctx.tradingAccountId).clear();
​
// copy active market ids for account being liquidated
ctx.activeMarketsIds = tradingAccount.activeMarketsIds.values();
​
// iterate over memory copy of active market ids
// intentionally not caching length as expected size < 3 in most cases
for (uint256 j; j < ctx.activeMarketsIds.length; j++) {
// load current active market id into working data
ctx.marketId = ctx.activeMarketsIds[j].toUint128();
​
​

And it didn't cancel all offchain orders

Impact

may result in an unintended position for the user

Example:
The current user position.size = 1000
User signs an offchain order targetPrice = 10e18 with sizeDelta = -1000
i.e., price is less than 10e18, position clears to 0

However, since it was liquidated, but the liquidation did not cancel all offline orders, when the user added collateral, this offchain order was executed accidentally
This resulted in the user's position.size = -1000, not the original clear 0

This is not the intended behavior of the user, resulting in a funding risk

Tools Used

Recommendations

add cancel all offchain

function liquidateAccounts(uint128[] calldata accountsIds) external {
...
​
​
// clear pending order for account being liquidated
MarketOrder.load(ctx.tradingAccountId).clear();
​
+ // bump the nonce value, which will invalidate all offchain orders signed with the previous nonce
+ unchecked {
+ tradingAccount.nonce++;
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`LiquidationBranch.liquidateAccounts` should cancel off-chain orders of the liquidated account.

Support

FAQs

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