Double Accounting of Liquidation Fees Leads to Inflated Balance Calculation and Solvency Bypass

Summary

Function LibSolvency::getAvailableBalanceAfterClosePosition() incorrectly calculates the available balance for Party A and Party B by double accounting for the liquidation fee (lf). This creates an artificial increase in the available balances of both parties, which could lead to the protocol losing funds, during position closure.

Root Cause

Look at the function: LibSolvency::getAvailableBalanceAfterClosePosition() it first calculates both parties' available balances, which is the party's allocated balance - lockedBalanceOfParty.cva + lockedBalanceOfParty.lf. These CVA (Credit Valuation Adjustment) and LF (Liquidation Fee) are the totals for all the quotes (positions) of the particular party (A or B). This means the partyBAvailableBalance and partyAAvailableBalance variables currently include the lf but not the cva. https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibSolvency.sol#L95-L111

Now if we go down, there is a for loop that processes all the quotes provided to the function. These quoteIds are the quote IDs of the quotes where the parties are partyA and partyB, and the loop iterates through all the quotes with the associated filledAmount, closedPrice, and marketPrice.

uint256 unlockedAmount = (filledAmount * (quote.lockedValues.cva + quote.lockedValues.lf)) / LibQuote.quoteOpenAmount(quote);
partyBAvailableBalance += int256(unlockedAmount);
partyAAvailableBalance += int256(unlockedAmount);

It again adds the lf to the partyBAvailableBalance and partyAAvailableBalance, which doubles the accounting of lf and increases the available balance of both parties.

Internal pre-conditions

The more will be the liquidationFee(lf) the more harm will caused by this bug

External pre-conditions

User or Attcker needs to call the forceClosePosition(), before his availableBalance + cva + lf + lf goes below 0. as we can see in the function LibSolvency::getAvailableBalanceAfterClosePosition()

Attack Path

Look at the Function PartyBPositionActionsFacet::fillCloseRequest() function. It makes a call to PartyBPositionActionsFacetImpl::fillCloseRequest(), which in turn calls LibSolvency::isSolventAfterClosePosition() to check if both parties are solvent after closing the position or not.

(int256 partyBAvailableBalance, int256 partyAAvailableBalance) = getAvailableBalanceAfterClosePosition(
    quoteIds,
    filledAmounts,
    closedPrices,
    marketPrices,
    upnlPartyB,
    upnlPartyA,
    partyB,
    partyA
);

With the returned available balance of Party A and Party B, it checks:

require(partyBAvailableBalance >= 0 && partyAAvailableBalance >= 0, "LibSolvency: Available balance is lower than zero");

Even if their balances are less than 0, this check will pass due to the double accounting of the lf. A similar issue will occur in ForceActionsFacetImpl::forceClosePosition() (there partyA can act maliciously), as it also uses this function.

Impact

The lf is typically between 0.1% to 0.5%, so if the amount is $10,000, the protocol could face a loss of $100 to $500 per transaction. This situation is not rare and can also be exploited by attackers, causing the protocol to lose funds.

So both partyA and partyB can act maliciously and can exploit using this.

PoC

No response

Mitigation

Correct the accounting error of 'lf' in the function LibSolvency::getAvailableBalanceAfterClosePosition()

sherlock-audit / 2024-09-symmio-v0-8-4-update-contest-judging

nikhil840096 - Double Accounting of Liquidation Fees Leads to Inflated Balance Calculation and Solvency Bypass #33