Ruhum - LiquidationFacet allows setPrice to be called with outdated prices

[sherlock-admin]

Ruhum

high

LiquidationFacet allows setPrice to be called with outdated prices

Summary

Vulnerability Detail

When prices are assigned to a symbol it only checks whether priceSig.timestamp <= maLayout.liquidationTimestamp[partyA] + maLayout.liquidationTimeout. It doesn't verify that the given price is recent, e.g. signed within the last 15 minutes.

    function setSymbolsPrice(address partyA, PriceSig memory priceSig) internal {
        MAStorage.Layout storage maLayout = MAStorage.layout();
        AccountStorage.Layout storage accountLayout = AccountStorage.layout();

        LibMuon.verifyPrices(priceSig, partyA);
        require(maLayout.liquidationStatus[partyA], "LiquidationFacet: PartyA is solvent");
        require(
            priceSig.timestamp <=
                maLayout.liquidationTimestamp[partyA] + maLayout.liquidationTimeout,
            "LiquidationFacet: Expired signature"
        );

Neither does the Muon library check whether the price is valid:

    function verifyPrices(PriceSig memory priceSig, address partyA) internal view {
        MuonStorage.Layout storage muonLayout = MuonStorage.layout();
        require(priceSig.prices.length == priceSig.symbolIds.length, "LibMuon: Invalid length");
        bytes32 hash = keccak256(
            abi.encodePacked(
                muonLayout.muonAppId,
                priceSig.reqId,
                address(this),
                partyA,
                priceSig.upnl,
                priceSig.totalUnrealizedLoss,
                priceSig.symbolIds,
                priceSig.prices,
                priceSig.timestamp,
                getChainId()
            )
        );
        verifyTSSAndGateway(hash, priceSig.sigs, priceSig.gatewaySignature);
    }

Thus, the liquidator is able to assign any price in the past for a given symbol. The price determines the payout to the other party on liquidation. While liquidators are currently permissioned, the protocol team has communicated that it's supposed to be permissionless in the future.

Impact

Liquidator can use outdated prices when liquidating party A.

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L38
https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/libraries/LibMuon.sol#L50

Tool used

Manual Review

Recommendation

Check whether a price is recent:
priceSig.timestamp > block.timestamp - 30 minutes

Duplicate of #113