Ethers sent to the contract can be stolen from any user

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L240-L266
https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L405-L442
https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L491-L505

Vulnerability details

Impact

User can by passing 0 as a _percentage, receive lpETH based on the ethers in the contract.

Proof of Concept

If the PrelaunchPoints.sol contract has native ether, any user can get lpETH for them without sending anything.

The user only needs to have a balance in the contract of the token that passes when he perform the attack to pass this check - it can be 1 wei.

uint256 userStake = balances[msg.sender][_token];
if (userStake == 0) {
    revert NothingToClaim();
}

User will call claim() with:

• _token - anyToken
• _percentage - 0
• Exchange - TransformERC20
• _data:
  • TRANSFORM_SELECTOR = 0x415565b0
  • inputToken = the token provided in _token
  • outputToken = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE
  • inputTokenAmount = 0

function claim(address _token, uint8 _percentage, Exchange _exchange, bytes calldata _data)

Then in _claim() it will enter the else statement, userClaim will become 0 because its - userStake * 0 / 100 = 0.

Then _validateData() will pass correctly, his balance will not be changed. When _fillQuote() is called, nothing will be exchanged because it will pass userClaim which is 0, so no ethers will enter the contract, and boughtETHAmount will remain the same. The claimedAmount will then be the contract balance and will be deposited into lpETH, by passing him as receiver, causing him to receive lpETH based on the Ether balance in the contract without having to sent anything**.**

PrelaunchPoints.sol#L240-L266

function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
    internal
    returns (uint256 claimedAmount)
{
    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);
}

PrelaunchPoints.sol#L405-L442

function _validateData(address _token, uint256 _amount, Exchange _exchange, bytes calldata _data) internal view {
    address inputToken;
    address outputToken;
    uint256 inputTokenAmount;
    address recipient;
    bytes4 selector;

    if (_exchange == Exchange.UniswapV3) {
        (inputToken, outputToken, inputTokenAmount, recipient, selector) = _decodeUniswapV3Data(_data);
        if (selector != UNI_SELECTOR) {
            revert WrongSelector(selector);
        }
        // UniswapV3Feature.sellTokenForEthToUniswapV3(encodedPath, sellAmount, minBuyAmount, recipient) requires `encodedPath` to be a Uniswap-encoded path, where the last token is WETH, and sends the NATIVE token to `recipient`
        if (outputToken != address(WETH)) {
            revert WrongDataTokens(inputToken, outputToken);
        }
    } else if (_exchange == Exchange.TransformERC20) {
        (inputToken, outputToken, inputTokenAmount, selector) = _decodeTransformERC20Data(_data);
        if (selector != TRANSFORM_SELECTOR) {
            revert WrongSelector(selector);
        }
        if (outputToken != ETH) {
            revert WrongDataTokens(inputToken, outputToken);
        }
    } else {
        revert WrongExchange();
    }

    if (inputToken != _token) {
        revert WrongDataTokens(inputToken, outputToken);
    }
    if (inputTokenAmount != _amount) {
        revert WrongDataAmount(inputTokenAmount);
    }
    if (recipient != address(this) && recipient != address(0)) {
        revert WrongRecipient(recipient);
    }
}

PrelaunchPoints.sol#L491-L505

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);
}

Tools Used

Manual Review

Recommended Mitigation Steps

I think the purpose of boughtETHAmount was to deposit in lpETH only if there is some swap that increases the native balance, so _claim() should be changed to deposit in lpETH only boughtETHAmount which is calculated in _fillQuote() this can be done by returning boughtETHAmount from _fillQuote() and then passing it to lpETH.deposit{value: boughtETHAmount}(_receiver).

Assessed type

Math

[c4-judge]

koolexcrypto marked the issue as duplicate of #33

[c4-judge]

koolexcrypto marked the issue as partial-25

[c4-judge]

koolexcrypto marked the issue as not a duplicate

[c4-judge]

koolexcrypto marked the issue as duplicate of #33

[c4-judge]

koolexcrypto marked the issue as partial-25