mahyar - AccountFacetImpl -> withdraw function doesn't check for balance

[sherlock-admin]

mahyar

high

AccountFacetImpl -> withdraw function doesn't check for balance

Summary

https://github.com/sherlock-audit/2023-06-symmetrical/blob/main/symmio-core/contracts/facets/Account/AccountFacetImpl.sol#L27 AccountFacetImpl library doesn't check the caller's balance to see if it's greater than the passed amount that they want to withdraw

Vulnerability Detail

In AccountFacet.sol contract user can call withdraw function and pass the amount they want to withdraw as an argument and the call will be forwarded to withdraw function of AccountFacetImpl.sol library to send the inputed amount to user But the function does not check if msg.sender has enough balance compare to the amount they passed; This means anyone without any balance can call withdraw and drain all of the tokens available inside the contract since there is no check for the balance.

Impact

1. Attacker call withdraw on AccountFacet contract and pass all of the amount available in contract and the contract forward the call to libarary's withdraw function
2. the function checks withdrawCooldown and deallocateCooldown but since they are 0 by default and there is no check for caller's balance it passes the first requirment and sends inputed amount to caller successfuly.

Code Snippet

    function withdraw(address user, uint256 amount) internal {
        AccountStorage.Layout storage accountLayout = AccountStorage.layout();
        GlobalAppStorage.Layout storage appLayout = GlobalAppStorage.layout();
        require(
            block.timestamp >=
            accountLayout.withdrawCooldown[msg.sender] + MAStorage.layout().deallocateCooldown,
            "AccountFacet: Cooldown hasn't reached"
        );
        uint256 amountWith18Decimals = (amount * 1e18) /
        (10 ** IERC20Metadata(appLayout.collateral).decimals());
        accountLayout.balances[msg.sender] -= amountWith18Decimals;
        IERC20(appLayout.collateral).safeTransfer(user, amount);
    }

Tool used

Manual Review

Recommendation

For fixing this problem you need to require caller's balance be higher or equal to the amount that he/she want to withdraw.

here is an example :

    function withdraw(address user, uint256 amount) internal {
        AccountStorage.Layout storage accountLayout = AccountStorage.layout();
        GlobalAppStorage.Layout storage appLayout = GlobalAppStorage.layout();
        uint256 amountWith18Decimals = (amount * 1e18) /
        (10 ** IERC20Metadata(appLayout.collateral).decimals());

        require(amountWith18Decimals <= accountLayout.balances[msg.sender],"insufficient funds")

        require(
            block.timestamp >=
            accountLayout.withdrawCooldown[msg.sender] + MAStorage.layout().deallocateCooldown,
            "AccountFacet: Cooldown hasn't reached"
        );

        accountLayout.balances[msg.sender] -= amountWith18Decimals;
        IERC20(appLayout.collateral).safeTransfer(user, amount);
    }