howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L257-L266 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L491-L505

Vulnerability details

Impact

When a user claims a non-ETH token, the _claim function incorrectly calculates the amount of lpETH to be minted and transferred to the user. If the contract holds any ETH balance before the token swap, this pre-existing ETH will be included in the lpETH minting, resulting in the user receiving more lpETH than they should. This could lead to inflation of the lpETH supply and loss of funds for the protocol.

Proof of Concept

The _claim function in the contract assumes that the entire ETH balance of the contract after a token-to-ETH swap is the result of that swap. This is incorrect if the contract already held some ETH before the swap. Here's the relevant part of the code: https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L257-L266

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

Let's consider the following scenario:

The contract holds a balance of 10 ETH from previous interactions.
A user calls the claim function to claim 1000 tokens of a non-ETH token, with a _percentage of 100.
The _claim function calculates userClaim as 1000 tokens and calls _fillQuote to swap these tokens for ETH.
The _fillQuote function performs the swap and receives 5 ETH in return.
After the swap, the contract's balance is now 15 ETH (10 ETH from before + 5 ETH from the swap).
The _claim function then calculates claimedAmount as the entire balance of the contract (15 ETH) and mints 15 lpETH to the user.

However, the user should only receive 5 lpETH, as that's the amount of ETH received from swapping their claimed tokens. The extra 10 ETH should not be included in the minting.

Tools used

Manual review

Recommended Mitigation Steps

Modify the _claim function to track the ETH balance before and after the swap, and only convert the difference to lpETH by adjusting the _fillQuote function to return the amount of ETH bought, and using this value in the _claim function:

// In _fillQuote function, return the bought ETH amount
function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal returns (uint256) {
    uint256 initialBalance = address(this).balance;
    require(_sellToken.approve(exchangeProxy, _amount));
    (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
    if (!success) {
        revert SwapCallFailed();
    }
    uint256 boughtETHAmount = address(this).balance - initialBalance;
    return boughtETHAmount;
}

// Adjust _claim function to use the returned value from _fillQuote
uint256 ethReceived = _fillQuote(IERC20(_token), userClaim, _data);
claimedAmount = ethReceived;
lpETH.deposit{value: claimedAmount}(_receiver);

Assessed type

ETH-Transfer