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

Wrong passing of input parameters in `liquidateAccounts` function

OdeWeb3

Summary

The liquidateAccounts parameters are being passed wrongly to the deductAccountMargin function which could lead unwanted consequences.

Vulnerability Details

The liquidateAccounts is a function meant to be called by the keepers in order to liquidate certain accounts that have gone below the required maintenance margin. Such accounts are said to be "under the water", hence are meant to be liquidated. While being liquidated, the orderFeeUsdX18 and settlementFeeUsdX18 are meant to be deducted from the account margin balance so as to reflect the true margin balance of the users account.

The settlementFeeUsdX18 is the total settlement fee to be deducted from the account while the orderFeeUsdX18 is the total order fee to be deducted from the account. This two variables are very unique to the protocol and serves two distinctive purpose, hence they should'nt be interchanged for one another.

But there is currently a mistake in how this variables are been passed to the deductAccountMargin function.
The deductAccountMargin aims to receive 5 different parameters as follows :-

function deductAccountMargin(
Data storage self,
FeeRecipients.Data memory feeRecipients,
UD60x18 pnlUsdX18,
UD60x18 settlementFeeUsdX18,
UD60x18 orderFeeUsdX18
)

However, the parameters being passed by the liquidateAccounts is as follows :-

ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
pnlUsdX18: requiredMaintenanceMarginUsdX18,
orderFeeUsdX18: UD60x18_ZERO, //@audit wrong passing of input parameters, the `deductAccountMargin` function takes settlement fee first before order fee
settlementFeeUsdX18: ctx.liquidationFeeUsdX18
});

As seen from above, there's a wrong passing of input parameters which in this case is unintended and could lead various consequences.

illustration

Supposing the liquidateAccounts is being called on a certain account which is currently "under the water" and liquidatable, the deductAccountMargin is meant to reflect the realized PnL of that account after the settlement fee and order fee have both been deducted. However, this wrong passing of input parameters could either understate or overstate the account PnL.

And as showcased above, the order Fee is meant to be 0 during liquidation.

  • orderFeeUsdX18: UD60x18_ZERO,

While settlement Fee is meant to be the liquidation fee

  • ctx.liquidationFeeUsdX18 = ud60x18(globalConfiguration.liquidationFeeUsdX18);

Although both settlementFeeUsdX18 and orderFeeUsdX18 have similar implementation in the deductions from margin, but this is an underlying error that could lead to various casualties in other implementations.

if (settlementFeeUsdX18.gt(UD60x18_ZERO) && ctx.settlementFeeDeductedUsdX18.lt(settlementFeeUsdX18)) {
// attempt to deduct from this collateral difference between settlement fee
// and amount of settlement fee paid so far
(ctx.withdrawnMarginUsdX18, ctx.isMissingMargin) = withdrawMarginUsd(
self,
collateralType,
ctx.marginCollateralPriceUsdX18,
settlementFeeUsdX18.sub(ctx.settlementFeeDeductedUsdX18),
feeRecipients.settlementFeeRecipient
);
​
// update amount of settlement fee paid so far by the amount
// that was actually withdraw from this collateral
ctx.settlementFeeDeductedUsdX18 = ctx.settlementFeeDeductedUsdX18.add(ctx.withdrawnMarginUsdX18);
}
​
// order fee logic same as settlement fee above
if (orderFeeUsdX18.gt(UD60x18_ZERO) && ctx.orderFeeDeductedUsdX18.lt(orderFeeUsdX18)) {
(ctx.withdrawnMarginUsdX18, ctx.isMissingMargin) = withdrawMarginUsd(
self,
collateralType,
ctx.marginCollateralPriceUsdX18,
orderFeeUsdX18.sub(ctx.orderFeeDeductedUsdX18),
feeRecipients.orderFeeRecipient
);
ctx.orderFeeDeductedUsdX18 = ctx.orderFeeDeductedUsdX18.add(ctx.withdrawnMarginUsdX18);
}

Impact

Break of logic and wrong passing of input parameters which could lead to overstating or understating an account's PnL

Tools Used

Manual Review

Recommendations

Change how the input parameters are been passed to how they are being received.

ctx.liquidatedCollateralUsdX18 = tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: address(0),
settlementFeeRecipient: globalConfiguration.liquidationFeeRecipient
}),
pnlUsdX18: requiredMaintenanceMarginUsdX18,
- orderFeeUsdX18: UD60x18_ZERO,
- settlementFeeUsdX18: ctx.liquidationFeeUsdX18
​
+ settlementFeeUsdX18: ctx.liquidationFeeUsdX18
+ orderFeeUsdX18: UD60x18_ZERO,
});
Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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