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

TradingAccount::deductAccountMargin() has incorrect fee parameter order, results in no incentive for liquidating an unhealthy position, accruing bad debt over time.

blutorque

Summary

Vulnerability Details

In LiquidationBranch.sol, trandingAccount.deductAccountMargin() is called to finalize the liquidation process. This function deducts the margin collateral balance of the liquidating account to collect the borrowed debt, and transfer a small % as a settlementFee to the liquidator.

File: LiquidationBranch.sol
https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/LiquidationBranch.sol#L152-L161

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

However, the issue with that is the settlementFeeUsdX18 order has been interchanged with orderFeeUsdX18. This results in the liquidator receiving 0 fees in exchange for removing bad debt from the protocol, which is wrong.

Here is the TradingAccount::deductAccountMargin() implementation,

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/TradingAccount.sol#L488-L493

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

The same function is used during the settlement of orders. For example, in SettlementBranch::_fillOrder(), it causes incorrect order fees/settlement fees to be sent to their respective recipients.

Impact

Due to wrong order in TradingAccount.deductAccountMargin() function, it has two critical impact

  1. Liquidator won't be incentivized from liquidating the unhealthy position, and their fee would be transferred to address(0)

  2. Incorrect order fee/settlement fee is being transferred

Tools Used

Manual review

Recommendations

function deductAccountMargin(
Data storage self,
FeeRecipients.Data memory feeRecipients,
UD60x18 pnlUsdX18,
- UD60x18 settlementFeeUsdX18,
- UD60x18 orderFeeUsdX18
+ UD60x18 orderFeeUsdX18,
+ UD60x18 settlementFeeUsdX18
)
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.

Give us feedback!