User can frontrun claiming and steal all of the locked ethers for himself

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L263

Vulnerability details

Description

According to the NatSpec comment of the receive() function: "ETH sent to this contract directly will be locked forever." - all ethers sent directly to the contract should be locked forever. However, ETH sent directly to the contract won't be locked forever but will be distributed among users who have locked ETH. The logic for distribution is insufficient though. A malicious user can frontrun the first user who tries to claim his tokens. The attacker claims a ERC20 token and this way the he will receive all of the locked ethers to himself.

        } 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;
            // @audit transfers the whole balance of the contract to the user
            ❌ lpETH.deposit{value: claimedAmount}(_receiver);
        }

Link

Impact

Malicious user can steal all other users' locked ethers.

Proof of Concept

• Multiple people send 1000 wei total directly to the contract, AFTER the locking period has ended
• Alice calls claim() for ETH and receives all the locked ethers.
• Alice has now stolen all of the locked ethers that should've been distributed among all users who have locked ETH.

Tools Used

Manual Review

Recommended Mitigation Steps

Calculate the amount of shares the user should receive instead of transferring the whole balance of the contract to the user.

Assessed type

ETH-Transfer

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[c4-judge]

koolexcrypto changed the severity to QA (Quality Assurance)

[c4-judge]

koolexcrypto marked the issue as grade-c