safdie - Logic error in `totalForPartyA()` and `totalForPartyB()` leads to inflated locked balances in `LibLockedValues.sol`

[sherlock-admin3]

safdie

High

Logic error in totalForPartyA() and totalForPartyB() leads to inflated locked balances in LibLockedValues.sol

Summary

Potential logic errors in totalForPartyA() and totalForPartyB() in LibLockedValues.sol leads to inflated locked balances.

Root Cause

The functions totalForPartyA() and totalForPartyB() aim to calculate the total locked balance for Party A and Party B, respectively. Here's a deeper look into the logic and the potential issue: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibLockedValues.sol#L81-L97 Both functions calculate the total locked balance by summing the cva, lf, and either partyAmm or partyBmm. However, the inclusion of cva and lf in both totals might lead to double-counting or inaccurate calculations. If cva (Collateral Value Adjustment) and lf (Liquidity Fund) are shared between the two parties, summing them for both parties could inflate the locked balances, causing the system to reflect incorrect values. This could lead to over-reporting of balances for both parties.

Internal pre-conditions

Imagine we have the following scenario:

LockedValues memory lv = LockedValues({
    cva: 1000,
    lf: 500,
    partyAmm: 2000,
    partyBmm: 1500
});

External pre-conditions

No response

Attack Path

Now, if we call totalForPartyA() and totalForPartyB():

uint256 partyATotal = totalForPartyA(lv); // 1000 + 2000 + 500 = 3500
uint256 partyBTotal = totalForPartyB(lv); // 1000 + 1500 + 500 = 3000

Both Party A and Party B are credited with their respective partyAmm or partyBmm values, but the cva and lf are being added to both totals.

Impact

1. The cva and lf values are added to both parties' totals, even though they may be shared resources or intended to be independent. This means the total locked balances for both Party A and Party B are overestimated. If these totals are used in financial settlement or collateral calculations, both parties could be seen as having more locked funds than they actually possess.
2. If cva and lf are intended to represent shared collateral or liquidity, they are double-counted. This could result in improper calculations for claims or liquidation events, leading to an imbalance or incorrect liquidation processes.
3. If this system is part of a larger financial protocol, malicious actors could exploit this incorrect calculation to misreport their locked funds. For example, Party A could claim they have more collateral than they actually do, possibly affecting loan terms, margin calls, or liquidation events in their favor. If the locked balances are used to calculate how much collateral is available for lending or borrowing, this overestimation could lead to under-collateralized positions and increased risk of insolvency or protocol failure.

PoC

No response

Mitigation

To fix this issue, you need to review how the cva and lf values should be treated in relation to the two parties. If these values should not be counted twice, then the totalForPartyA() and totalForPartyB() functions need to be adjusted.

One potential fix:

function totalForPartyA(LockedValues memory self) internal pure returns (uint256) {
    return self.partyAmm; // Exclude `cva` and `lf` if they are shared between parties
}

function totalForPartyB(LockedValues memory self) internal pure returns (uint256) {
    return self.partyBmm; // Exclude `cva` and `lf` if they are shared between parties
}