0xChinedu - Token Amount is scaled up twice in depositAndAllocate() causing the amount to be inflated

[sherlock-admin]

0xChinedu

high

Token Amount is scaled up twice in depositAndAllocate() causing the amount to be inflated

Summary

In AccountFacet.depositAndAllocate() the token amount to be allocated is scaled up twice which causes the amount to be inflated.

Vulnerability Detail

The depositAndAllocate() allows Party A to deposit and allocate the amount they wish to engage in trading. However, the said amount deposited is first scaled when depositAndAllocate() calls AccountFacetImpl.deposit() with the said amount (and msg.sender) and deposits it into Party A's wallet.
Then scaled again in depositAndAllocate() before passing it into AccountFacetImpl.allocate() to be allocated. As such, any token (e.g. USDC) with a decimal of less than 1e18 will be scaled up twice. This will cause the allocatedBalances[msg.sender] to be inflated.
https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacet.sol#L46-L55

    function depositAndAllocate(
        uint256 amount
    ) external whenNotAccountingPaused notLiquidatedPartyA(msg.sender) {
        AccountFacetImpl.deposit(msg.sender, amount);
        uint256 amountWith18Decimals = (amount * 1e18) /
        (10 ** IERC20Metadata(GlobalAppStorage.layout().collateral).decimals());
        AccountFacetImpl.allocate(amountWith18Decimals);
        emit Deposit(msg.sender, msg.sender, amount);
        emit AllocatePartyA(msg.sender, amountWith18Decimals);
    }

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L19-L25

    function deposit(address user, uint256 amount) internal {
        GlobalAppStorage.Layout storage appLayout = GlobalAppStorage.layout();
        IERC20(appLayout.collateral).safeTransferFrom(msg.sender, address(this), amount);
        uint256 amountWith18Decimals = (amount * 1e18) /
        (10 ** IERC20Metadata(appLayout.collateral).decimals());
        AccountStorage.layout().balances[user] += amountWith18Decimals;
    }

Impact

Accounting of funds allocated to Party A will be inflated as a result of multiple scaling.

Code Snippet

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacet.sol#L46-L55
https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L19-L25

Tool used

Manual Review

Recommendation

Amount to be allocated by Party A should not be scaled again in depositAndAllocate() after it has already been scaled in AccountFacetImpl.deposit().

Duplicate of #222