Attackers could steal the ETH contained in the PrelaunchPoints contract

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

When users claim lpETH and the specified _token is not ETH, if there is ETH in the PrelaunchPoints contract transferred by mistake by others, the contract will also convert this part of ETH into lpETH for the user. This is because the amount of lpETH calculated for the user's claim in the _claim function is address(this).balance, rather than the actual amount of ETH obtained through exchangeProxy with the _token in the _fillQuote function.

_fillQuote(IERC20(_token), userClaim, _data);

// Convert swapped ETH to lpETH (1 to 1 conversion)
claimedAmount = address(this).balance;
lpETH.deposit{value: claimedAmount}(_receiver);

Although the comment states, "At this point there should not be any ETH in the contract," this is an ideal situation. In reality, it is highly possible that someone could mistakenly transfer ETH to the PrelaunchPoints contract.

Proof of Concept

1. User A calls the lock function, locking an amount of _token into the PrelaunchPoints contract.
2. The owner calls the convertAllETH function to convert the ETH in the PrelaunchPoints contract into lpETH and updates the startClaimDate variable, allowing users who have locked assets in the contract to start claiming lpETH.
3. User B mistakenly transfers 10 ETH to the PrelaunchPoints contract.
4. User A observes User B's mistaken transfer and immediately calls the claim function to claim lpETH. The claim function calls the _claim function for the actual operation. In the _claim function, _fillQuote is invoked, which converts User A's _token into ETH held within the PrelaunchPoints contract.
5. The contract converts the amount of ETH denoted as claimedAmount into lpETH for User A’s specified _receiver. Note that the claimedAmount here is address(this).balance, which includes User B's 10 ETH, not just the boughtETHAmount calculated in the final _fillQuote function.
6. User A successfully steals User B's 10 ETH.

Tools Used

None

Recommended Mitigation Steps

1. It is suggested that the _fillQuote function return the final calculated boughtETHAmount to the _claim function. In the _claim function, convert the amount of ETH denoted as boughtETHAmount into lpETH for the _receiver.
2. It is recommended to implement a revert logic in the receive function to prevent users from mistakenly transferring ETH into the contract.

Assessed type

Other

[c4-judge]

koolexcrypto marked the issue as duplicate of #18

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto marked the issue as partial-50

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[c4-judge]

koolexcrypto marked the issue as duplicate of #33