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

0 stars 0 forks source link

air_0x - AccountFacetImpl.sol:withdraw allows draining of funds #34

Open sherlock-admin3 opened 1 week ago

sherlock-admin3 commented 1 week ago

air_0x

High

AccountFacetImpl.sol:withdraw allows draining of funds

Summary

The withdraw function deducts an 18-decimal scaled amount from the user's balance but transfers the original, unscaled amount of tokens, potentially allowing users to withdraw more funds than they actually possess, which could lead to draining the contract's funds

Root Cause

The oversight in the withdraw function as it subtracts amountWith18Decimals from the user's balance but transfers the unscaled amount to the user as shown below allows draining of the protocol since users can specify any amount

    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"
        );
//@audit-info :  calculates the scaled amount
        uint256 amountWith18Decimals = (amount * 1e18) / (10 ** IERC20Metadata(appLayout.collateral).decimals());
        accountLayout.balances[msg.sender] -= amountWith18Decimals;
    //@audit-issue : transfers the unscaled amount to user 
        IERC20(appLayout.collateral).safeTransfer(user, amount);
    }

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

draining of protocol

PoC

No response

Mitigation

function withdraw(address user, uint256 amount) internal {
.......
  - IERC20(appLayout.collateral).safeTransfer(user, amount);
  +IERC20(appLayout.collateral).safeTransfer(user, amountWith18Decimals);
}