Denial of Service in Internal Transfer Function Due to PartyB Modifier

Summary

The internalTransfer function is intended to allow transfers to partyB, but the presence of the userNotPartyB modifier prevents this action, causing a denial of service (DoS) for legitimate transfers to other partyB accounts.

According to the updated Symmio 0.8.4, , Internal transfer have been allowed for partyBs. With the new update, partyBs have balance managers expected to constantly monitor and carry Out allocations and dellocations for their users, but due to the continued presence of the modifier, Internal transfer remains exclusive to party A users only which is in complete rebuttal to the updated docs.

Root Cause

The root cause of the vulnerability is the userNotPartyB(user) modifier, which prevents the function from executing if the user is partyB. This contradicts the intended functionality of allowing transfers to partyB.

Internal pre-conditions

-The internalTransfer function must be called with a user address that is partyB. -The whenNotInternalTransferPaused condition must be met. -The msg.sender and user must not be suspended or liquidated.

Attack Path

-A user attempts to call the internalTransfer function to transfer funds to partyB. -The function checks the userNotPartyB(user) modifier. -The check fails because the user is partyB. -The transaction reverts, preventing the transfer from occurring.

Impact

The aim of the function is to allow partyB users to transfer some of their funds to a different accounts without waiting for Deallocate cooldown. Due to error partyB is denied an expected service and mandated to wait until Deallocate cool down before transferring their funds to other accounts

PoC

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/Account/AccountFacet.sol#L81-L91

Here is the code with unexpected modifier

    function internalTransfer(
        address user,
        uint256 amount
    ) external whenNotInternalTransferPaused userNotPartyB(user) notSuspended(msg.sender) notSuspended(user) notLiquidatedPartyA(user) {
        AccountFacetImpl.internalTransfer(user, amount);
        emit InternalTransfer(msg.sender, user, AccountStorage.layout().allocatedBalances[user], amount);
        emit Withdraw(msg.sender, user, ((amount * (10 ** IERC20Metadata(GlobalAppStorage.layout().collateral).decimals())) / (10 ** 18)));
        emit AllocatePartyA(user, amount, AccountStorage.layout().allocatedBalances[user]);
        emit AllocatePartyA(user, amount); // For backward compatibility, will be removed in future
        emit SharedEvents.BalanceChangePartyA(user, amount, SharedEvents.BalanceChangeType.ALLOCATE);
    }

Mitigation

Remove the userNotPartyB modifier or adjust its logic to allow transfers to partyB while still preventing unauthorized actions

sherlock-audit / 2024-09-symmio-v0-8-4-update-contest-judging

Aycozzynfada - Denial of Service in Internal Transfer Function Due to PartyB Modifier #59