howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L392 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L259-L263 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L503-L504 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L321-L324

Vulnerability details

Impact

If a user mistakenly sends ETH to the contract before convertAllETH has been called by the owner, every user could claim more lpETH than expected. If he mistakenly sends ETH to the contract after convertAllETH has been called by the owner, the first user to claim after him could get more lpETH.

Proof of Concept

The contract is able to receive ETH.

    /**
     * Enable receive ETH
     * @dev ETH sent to this contract directly will be locked forever.
     */
    receive() external payable {}

However, if a user mistakenly sends ETH to the contract, this would affect the claiming process.

Consider the following 2 cases:

If a user mistakenly sends ETH to the contract before convertAllETH has been called by the owner, every user could claim more lpETH than expected.

When convertAllETH is being called by the owner, all ETH in the contract will be converted to lpETH. When a user claims his locked ETH afterwards, the calculation claimedAmount = userStake.mulDiv(totalLpETH, totalSupply) will give him more lpETH than actual amount.

        uint256 totalBalance = address(this).balance;
        lpETH.deposit{value: totalBalance}(address(this));
        totalLpETH = lpETH.balanceOf(address(this));

If he mistakenly sends ETH to the contract after convertAllETH has been called by the owner, the first user to claim after him could get more lpETH.

When _claim is later being called, lpETH.deposit{value: claimedAmount}(_receiver) has been called to convert all ETH in the contract to lpETH. Thus the first user to claim afterwards could get more lpETH than actual amount.

            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);

Tools Used

Manual, VSCode

Recommended Mitigation Steps

Three steps:

Since totalSupply is used to record ETH balance, it is better to use totalSupply to convert to lpETH.
Use the boughtETHAmount obtained in _fillQuote to convert to ETH.
Add a retrieve function to retrieve address(this).balance-totalSupply before claiming and address(this).balance after claiming.

Assessed type

ETH-Transfer