rvierdiiev - LiquidationFacetImpl.liquidatePositionsPartyB doesn't check provided signature timestamp correctly

[sherlock-admin]

rvierdiiev

medium

LiquidationFacetImpl.liquidatePositionsPartyB doesn't check provided signature timestamp correctly

Summary

LiquidationFacetImpl.liquidatePositionsPartyB doesn't check provided signature timestamp correctly

Vulnerability Detail

When partyB is liquidated, then partyBLiquidationTimestamp is set to it.
Later, liquidatePositionsPartyB function can be called to close positions for partyB. Liquidator provides QuotePriceSig to this call.

There are 2 checks of priceSig that are going to validate if it's provided for correct timestamp.
https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L318-L322

        require(
            priceSig.timestamp <=
                maLayout.partyBLiquidationTimestamp[partyB][partyA] + maLayout.liquidationTimeout,
            "LiquidationFacet: Expired signature"
        );

This is first one and it checks that priceSig is not more in future then liquidation time + maLayout.liquidationTimeout.

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L327-L330

        require(
            block.timestamp <= priceSig.timestamp + maLayout.liquidationTimeout,
            "LiquidationFacet: Expired price sig"
        );

This is another one, which checks that current time is not bigger than priceSig + maLayout.liquidationTimeout.

In order to provide valid priceSig it should:
1.priceSig.timestamp <= maLayout.partyBLiquidationTimestamp[partyB][partyA] + maLayout.liquidationTimeout
2.block.timestamp - maLayout.liquidationTimeout > priceSig.timestamp
then we have
block.timestamp - maLayout.liquidationTimeout > priceSig.timestamp <= maLayout.partyBLiquidationTimestamp[partyB][partyA] + maLayout.liquidationTimeout

This condition is actually incorrect, because it depends on block.timestamp, which means that if liquidator will not call this function in this time period, then he will not be able to call it later.

Correct condition should be:
maLayout.partyBLiquidationTimestamp[partyB][partyA] >= priceSig.timestamp <= maLayout.partyBLiquidationTimestamp[partyB][partyA] + maLayout.liquidationTimeout

This means that liquidator should provide signature that was created between maLayout.partyBLiquidationTimestamp[partyB][partyA] and maLayout.partyBLiquidationTimestamp[partyB][partyA] + maLayout.liquidationTimeout period.

Impact

Liquidator will not be able to call function.

Code Snippet

Provided above

Tool used

Manual Review

Recommendation

Use this condition to allow signature:
maLayout.partyBLiquidationTimestamp[partyB][partyA] >= priceSig.timestamp <= maLayout.partyBLiquidationTimestamp[partyB][partyA] + maLayout.liquidationTimeout

Duplicate of #113