safdie - Inconsistent liquidation state could result in incorrect liquidation handling or an incomplete process in `DeferredLiquidationFacetImpl.sol`

[sherlock-admin3]

safdie

Medium

Inconsistent liquidation state could result in incorrect liquidation handling or an incomplete process in DeferredLiquidationFacetImpl.sol

Summary

Inconsistent liquidation state in DeferredLiquidationFacetImpl.sol could result in incorrect liquidation handling or an incomplete process.

Root Cause

The inconsistent liquidation state vulnerability can be tested by crafting a PoC below to demonstrate how calling deferredSetSymbolsPrice before deferredLiquidatePartyA (or vice versa) could result in incorrect liquidation handling or an incomplete process. https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/liquidation/DeferredLiquidationFacetImpl.sol#L22-L99

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. Setup the initial liquidation state: Call deferredLiquidatePartyA to set the liquidation status for partyA.
2. Manipulate the state with out-of-order calls: Call deferredSetSymbolsPrice without completing or checking the liquidation process first.
3. Result observation: Check if the liquidation has unintended consequences, like being able to alter the liquidation type or incorrectly set symbols' prices while partyA is not fully liquidated.

Impact

1. Liquidation might fail to process properly, leading to discrepancies in balances for partyA. This could allow partyA to avoid or delay liquidation unfairly.
2. Incorrect prices or liquidation states could lead to misinformed decisions, causing further financial impact on parties involved.
3. A malicious actor could intentionally perform liquidation actions out of order to exploit inconsistencies and gain financially.

PoC

// SPDX-License-Identifier: MIT
pragma solidity >=0.8.18;

import "./DeferredLiquidationFacetImpl.sol";

contract TestInconsistentLiquidationState {
    function testOutOfOrderLiquidation(address partyA, DeferredLiquidationSig calldata liquidationSig) external {
        // Step 1: Call deferredSetSymbolsPrice before deferredLiquidatePartyA
        DeferredLiquidationFacetImpl.deferredSetSymbolsPrice(partyA, liquidationSig);

        // Step 2: Now call deferredLiquidatePartyA, which should ideally trigger liquidation, 
        // but the liquidation status and symbol prices may be inconsistently set
        DeferredLiquidationFacetImpl.deferredLiquidatePartyA(partyA, liquidationSig);

        // Check: Observe the liquidation status and symbol prices for `partyA`
        // Ideally, there should be logs or checks to validate if the state is inconsistent
    }
}

Mitigation

To prevent this, implement a state machine pattern in deferredLiquidatePartyA and deferredSetSymbolsPrice that enforces the correct order of operations. This pattern ensures that deferredLiquidatePartyA must complete before deferredSetSymbolsPrice can execute, and vice versa.