sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

simon135 - There is no way for the system to make sure the user Already got its pnl and take some action on it in `partyAAvailableForQuote` #351

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

simon135

medium

There is no way for the system to make sure the user Already got its pnl and take some action on it in partyAAvailableForQuote

Summary

Since in this Function, we evaluate unpl as PartyA total unpl here is a way how PartyA can make a lot of positions on continuing pnl based on delta neutral position and take up a lot of the liquidity and requests

Vulnerability Detail

steps: PartyA makes 5 eth positions 2 long and 3 short and let's say eth falls 5 percent-> pnl=1000 PartyA makes another of the same 5 positions allocated=1 + 1000 - 300= 701 and we can create more positions and now pnl=2000 PartyA makes another 5 positions allocated 2000-1000=1000 units of tokens for positions and again and again to keep this up the PartyA just needs pnl which can be acquired from on-chain price movement and in that moment they can create these many positions The root cause is that pnl can keep growing with no check on how many positions are being taken It's medium because it requires an unlikely scenario where PartyA gets lucky in pnl and the price keeps that way so he leverages the pnl and the system never checks that pnl is already used like other defi systems have.

Impact

Loss of funds and no check in the state less unpl that has no check if used from the smart contracts at least

Code Snippet

none just design issue that causes over leveraging and unfair advantage

        AccountStorage.Layout storage accountLayout = AccountStorage.layout();
        int256 available;
        if (upnl >= 0) {
            available =
                int256(accountLayout.allocatedBalances[partyA]) +
                upnl -
                int256(
                    (accountLayout.lockedBalances[partyA].total() +
                        accountLayout.pendingLockedBalances[partyA].total())
                );
        } else {
            int256 mm = int256(accountLayout.lockedBalances[partyA].mm);
            int256 considering_mm = -upnl > mm ? -upnl : mm;
            available =
                int256(accountLayout.allocatedBalances[partyA]) -
                int256(
                    (accountLayout.lockedBalances[partyA].cva +
                        accountLayout.lockedBalances[partyA].lf +
                        accountLayout.pendingLockedBalances[partyA].total())
                ) -
                considering_mm;
        }
        return available;
    }

https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/libraries/LibAccount.sol#L30

Tool used

Manual Review

Recommendation

add another part of the sig that checks that pnl can only be used once in the system

bool pnlUsed=True
//@reason: in next call that PartyA makes  it will revert not allowing him total unfair access to pnl 
require(pnlUsed==false)