code-423n4 / 2024-05-loop-validation

0 stars 0 forks source link

Users can not withdraw their `WETH` funds that they deposited #202

Closed c4-bot-2 closed 6 months ago

c4-bot-2 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L133 https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L157 https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L172 https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L191 https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L274

Vulnerability details

Impact

Users can not withdraw WETH and users can lose their token deposits. This can cause a denial of services if users try to make a withdraw and they can't withdraw their WETH deposits .User WETH funds will be stuck in the contract forever.

Proof of Concept

if a user locks their WETH the _processLock function maps the user that they deposited ETH not WETH in this line https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L191

balances[_receiver][ETH] += _amount

The balances variable will never have a user that is mapped to WETH even if they deposited more WETH funds.

If a user calls the withdraw function with the WETH address the withdraw function will not Transfer any funds and revert or Transfer zero funds to the user , because balances does not map WETH tokens even if they are deposited

Tools Used

manual

Recommended Mitigation Steps

change line 191 from : balances[_receiver][ETH] += _amount to this code balances[_receiver][WETH] += _amount

      function _processLock(address _token, uint256 _amount, address _receiver, bytes32 _referral)
        internal
        onlyBeforeDate(loopActivation)
    {
        if (_amount == 0) {
            revert CannotLockZero();
        }
        if (_token == ETH) {
            totalSupply = totalSupply + _amount;
            balances[_receiver][ETH] += _amount;
        } else {
            if (!isTokenAllowed[_token]) {
                revert TokenNotAllowed();
            }
            IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);

            if (_token == address(WETH)) {
                WETH.withdraw(_amount);
                totalSupply = totalSupply + _amount;
-                balances[_receiver][ETH] += _amount;
+                balances[_receiver][WETH] += _amount;
            } else {
                balances[_receiver][_token] += _amount;
            }
        }

        emit Locked(_receiver, _amount, _token, _referral);
    }

Assessed type

Token-Transfer

0xSorryNotSorry commented 6 months ago

It´s the intended mechanism (WETH deposits increases the ETH balance due to conversion and it´s the first audit issue which is mitigated to existing code - so it´s on purpose)

No funds lost as they can either withdraw ETH (guided by the frontend) or claim lpETH after the claimdate

0xSorryNotSorry commented 6 months ago

@howlbot reject