search

sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

simon135 - An attacker can create a position with just UPNL which should not be allowed in `sendQuote` #333

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

simon135

high

An attacker can create a position with just UPNL which should not be allowed in sendQuote

Summary

An attacker can create a position with just UPNL which should not be allowed

Vulnerability Detail

The invariant breaks that a position needs to have collateral because if the Upnl is positive then PartyA is allowed to make that position without collateral and if the pnl suddenly drops after then they do not have that collateral steps:

  1. Alice has position with PartyB(bob) and he has pnl of 1000 units of tokens
  2. Alice creates position with 1000 units of pnl
  3. Alice losses his first position but he still have 2 position with no collateral backed

    Impact

    can cause uncollateralized positions and brakes the main invariant that we need to deposit collateral/allocate

    Code Snippet

    // @audit as you can see balance is never checked that we have collateral but have pnl 
if (upnl >= 0) { 
        available =
            int256(accountLayout.allocatedBalances[partyA]) +
            upnl -
            int256(
                (accountLayout.lockedBalances[partyA].total() +
                    accountLayout.pendingLockedBalances[partyA].total())
            );

    https://github.com/sherlock-audit/2023-06-symmetrical/blob/6d2b64b6732fcfbd07c8217897dd233dbb6cd1f5/symmio-core/contracts/facets/PartyA/PartyAFacet.sol#L13

    Tool used

Manual Review

Recommendation

check the balance of PartyA and don't just rely on pnl for opening positions because an attacker can take advantage of this Don't use Upnl in determining if the PartyA has enough Available.Have a second function that adds the pnl to the allocatedBalances or that user has positions

Vulnerability Detail