sherlock-audit / 2023-06-symmetrical-judging

5 stars 4 forks source link

AkshaySrivastav - `AccountFacet.depositAndAllocateForPartyB` is missing `notLiquidatedPartyB` modifier #308

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

AkshaySrivastav

medium

AccountFacet.depositAndAllocateForPartyB is missing notLiquidatedPartyB modifier

Summary

AccountFacet.depositAndAllocateForPartyB is missing notLiquidatedPartyB modifier. Hence allows allocating funds towards a liquidated partyB-partyA pair.

Vulnerability Detail

The AccountFacet.allocateForPartyB function has the notLiquidatedPartyB modifier. But that validation can be bypassed by calling depositAndAllocateForPartyB function.

    function allocateForPartyB(
        uint256 amount,
        address partyA
    ) public whenNotPartyBActionsPaused notLiquidatedPartyB(msg.sender, partyA) onlyPartyB {
        AccountFacetImpl.allocateForPartyB(amount, partyA, true);
        emit AllocateForPartyB(msg.sender, partyA, amount);
    }

    function depositAndAllocateForPartyB(
        uint256 amount,
        address partyA
    ) external whenNotPartyBActionsPaused onlyPartyB {
        AccountFacetImpl.depositForPartyB(amount);
        AccountFacetImpl.allocateForPartyB(amount, partyA, true);
        emit DepositForPartyB(msg.sender, amount);
        emit AllocateForPartyB(msg.sender, partyA, amount);
    }

Impact

Using depositAndAllocateForPartyB a partyB can allocate funds towards a liquidated partyB-partyA pair. This can severely impact liquidations as the allocated balance will get changed between the liquidatePartyB and liquidatePositionsPartyB functions.

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacet.sol#L66-L82

Tool used

Manual Review

Recommendation

Consider adding the notLiquidatedPartyB modifier to the AccountFacet.depositAndAllocateForPartyB function.

Duplicate of #247