sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

simon135 - no check for exipred Price Timestamp like in PartyB which can cause price staleness #320

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

simon135

high

no check for exipred Price Timestamp like in PartyB which can cause price staleness

Summary

In PartyB liquidation priceSig.timestamp is checked to be close to block.timstamp but in PartyA liquidation this is not checked which can cause some issues

Vulnerability Detail

The second step in liquidation for PartyA

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

Then for setSymbolsPrices https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/facets/liquidation/LiquidationFacetImpl.sol#L34 it doesn't check the sig for timestamp forverifyPrice` https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/libraries/LibMuon.sol#L34C5-L63C16 so A liquidator can use an old price from a long timestamp and since its not checked it can cause profit for PartyA/PartyB and this can be dirrected at each party causing some of them loss Ex: Liquidation happens at eth price 1000 at 20 timestamp Now price is at 2000 at timestamp 40 but 20 is still used and since there is no check PartyA has short So PartyB can get more profit than they should

Impact

Free profit for PartyB/PartyA

Code Snippet

function setSymbolsPrice(address partyA, PriceSig memory priceSig) internal {
        MAStorage.Layout storage maLayout = MAStorage.layout();
        AccountStorage.Layout storage accountLayout = AccountStorage.layout();
// @audit this dosnt check the timestmap like the other muon functions 
        LibMuon.verifyPrices(priceSig, partyA);
        require(maLayout.liquidationStatus[partyA], "LiquidationFacet: PartyA is solvent");
// @audit see here that chec is not good since timestmap can be below these      
   require(
            priceSig.timestamp <=
                maLayout.liquidationTimestamp[partyA] + maLayout.liquidationTimeout,
            "LiquidationFacet: Expired signature"
        );
        for (uint256 index = 0; index < priceSig.symbolIds.length; index++) {
            accountLayout.symbolsPrices[partyA][priceSig.symbolIds[index]] = Price(
                priceSig.prices[index],
                maLayout.liquidationTimestamp[partyA]
            );
        }

        int256 availableBalance = LibAccount.partyAAvailableBalanceForLiquidation(
            priceSig.upnl,
            partyA
        );
        if (accountLayout.liquidationDetails[partyA].liquidationType == LiquidationType.NONE) {
            accountLayout.liquidationDetails[partyA] = LiquidationDetail({
                liquidationType: LiquidationType.NONE,
                upnl: priceSig.upnl,
                totalUnrealizedLoss: priceSig.totalUnrealizedLoss,
                deficit: 0,
                liquidationFee: 0
            });

Tool used

Forge Manual Review

Recommendation

Add timestmap check to muon or this:

 block.timestamp <= priceSig.timestamp + maLayout.liquidationTimeout, 

Duplicate of #113