after `_fillQuote` the contract transfer wrong `ETH` amount

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L262
https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L491-L505

Vulnerability details

Impact

The user could receive the forever locked ETH as part of claim ETH.

Proof of Concept

The PrelaunchPoints contract has receive() function and the comment on it says that:

@dev ETH sent to this contract directly will be locked forever.
However inside the claim() function the contract check that if the out token is not ETH then it will first convert it into ETH for this it will call _fillQuote():

function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal {
        // Track our balance of the buyToken to determine how much we've bought.
      @>  uint256 boughtETHAmount = address(this).balance;

        require(_sellToken.approve(exchangeProxy, _amount));

        (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
        if (!success) {
            revert SwapCallFailed();
        }

        // Use our current buyToken balance to determine how much we've bought.
     @>   boughtETHAmount = address(this).balance - boughtETHAmount;
        emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
    }

Here the contract calls the exchangeProxy and convert token to ETH and subtract the received ETH from before ETH balance.
inside the _claim function :

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

            // Convert swapped ETH to lpETH (1 to 1 conversion)
  @>          claimedAmount = address(this).balance; // @audit : it will give all the ETH of contract to receiver
  @>         lpETH.deposit{value: claimedAmount}(_receiver);

Here the contract transfer the complete ETH Balance to the _receiver.
The Issue here is that this function will also transfer the locked ETH to the receiver.

let assume that the contract have 1 ETH locked.

1. Alice have 1 stETH to claim.
2. Alice calls the claim function to claim her token.
3. The contract call the _fillQuote function to convert the assets.
4. The _fillQuote convert 1 stEth to ETH and receive 1 ETH after conversion. assume 1:1 ETH:stETH.
5. The claim function get the balance of contract which is 1 Locked ETH + 1 received ETH after conversion so the address(this).balance = 2 ETH.
6. The contract transfer 2 ETH to Alice.

Tools Used

Manual Review

Recommended Mitigation Steps

Extract the exact balance received after conversion and transfer it to receive . because the user is not allow to receive the Locked ETH in contract.
one solution cold be as follows:
return the boughtAmount from _fillQuote function and send it to the receiver.

diff --git a/src/PrelaunchPoints.sol b/src/PrelaunchPoints.sol
index aa6b9f4..a7fc49f 100644
--- a/src/PrelaunchPoints.sol
+++ b/src/PrelaunchPoints.sol
@@ -253,13 +253,12 @@ contract PrelaunchPoints {
             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);
+          claimedAmount=  _fillQuote(IERC20(_token), userClaim, _data);
 
             // Convert swapped ETH to lpETH (1 to 1 conversion)
-            claimedAmount = address(this).balance;
             lpETH.deposit{value: claimedAmount}(_receiver);

@@ -488,7 +487,7 @@ contract PrelaunchPoints {
      * @param _swapCallData  The `data` field from the API response.
      */
 
-    function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal {
+    function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal returns(uint256 ){
         // Track our balance of the buyToken to determine how much we've bought.
         uint256 boughtETHAmount = address(this).balance;
 
@@ -501,6 +500,7 @@ contract PrelaunchPoints {
 
         // Use our current buyToken balance to determine how much we've bought.
         boughtETHAmount = address(this).balance - boughtETHAmount;
+        return boughtETHAmount;
         emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
     }

Assessed type

ETH-Transfer

[c4-judge]

koolexcrypto marked the issue as duplicate of #18

[c4-judge]

koolexcrypto marked the issue as partial-75

[koolexcrypto]

contract have 1 ETH locked

Didn't explain how the contract got 1 ETH locked, the contract shouldn't have any ETH locked unless someone sent to it directly.

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[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

[c4-judge]

koolexcrypto marked the issue as not a duplicate

[c4-judge]

koolexcrypto changed the severity to QA (Quality Assurance)

[c4-judge]

koolexcrypto marked the issue as grade-c