Claims that are not paused during `emergencymode` can cause users to get more `lpETH` than the amount of `ETH` they have locked

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L240-L266

Vulnerability details

Impact

Users get more lpETH than the amount of ETH they have locked

Proof of Concept

The LoopFi protocol has an emergency mode feature which allows users to withdraw their assets even though the loopActivation and startClaimDate periods have passed. However, in emergency mode, users can still claim lpETH by exchanging the ETH they have with the claim() function. This can be a problem because the total supply of ETH changes and affects the exchange rate of lpETH and ETH.

Exploit Scenario

1. Normally, the user initially locks his ETH with the lockETH() or lockETHfor() function until a certain time limit.
2. Then after a certain time, owner calls the setLoopAddresses() function and starts the first epoch. And after 7 days the owner calls the convertAllETH() function to start the claim period for users to be able to claim lpETH.
3. For example, it is assumed that the data obtained:
   • totalLpETH : 1000
   • totalSupply : 1000
4. Because there was a problem, the owner activated the emergencyMode feature by calling the setEmergencyMode() function
5. We assume that some users withdraw their ETH and the data obtained becomes:
   • totalLpETH : 1000
   • totalSupply : 500
6. Bob knows this condition and has a locked ETH balance (i.e. 100 ETH) calls the claim() function to exchange it for lpETH.

        if (_token == ETH) {
            claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
            balances[msg.sender][_token] = 0;
            lpETH.safeTransfer(_receiver, claimedAmount);

7. With the formula above, Bob should have gotten 100 lpETH tokens but now he gets 200 lpETH tokens because the exchange rate has changed due to the change in the total supply of ETH.

NOTE :

1. This also affects the claimAndStake() function

2. If this condition occurs, the main variant will be broken

Users that deposit ETH/WETH get the correct amount of lpETH on claim (1 to 1 conversion)

3. Another user after the first claimer will get less lpETH than they should

Tools Used

Manual review

Recommended Mitigation Steps

Consider pause claim() function if protocol on emergencyMode state

    function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        internal
        returns (uint256 claimedAmount)
    {
        if (emergencyMode) {
            revert CurrentlyNotPossible();
    }

        uint256 userStake = balances[msg.sender][_token];
        if (userStake == 0) {
            revert NothingToClaim();
        }
        if (_token == ETH) {
            claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
            balances[msg.sender][_token] = 0;
            lpETH.safeTransfer(_receiver, claimedAmount);
        } else {
            uint256 userClaim = userStake * _percentage / 100;
            _validateData(_token, userClaim, _exchange, _data);
            balances[msg.sender][_token] = userStake - userClaim;

            // At this point there should not be any ETH in the contract
            // Swap token to ETH
            _fillQuote(IERC20(_token), userClaim, _data);

            // Convert swapped ETH to lpETH (1 to 1 conversion)
            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);
        }
        emit Claimed(msg.sender, _token, claimedAmount);
    }

Assessed type

Other

[0xd4n1el]

This scenario is not possible since withdraw cannot happen after convertAllETH

[c4-judge]

koolexcrypto marked the issue as primary issue

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Invalid