howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The function _claim when _token != ETH assumes that the entire amount specified by the user will always be entirely swapped when interacting with the Uniswap V3 pool. However, due to price limits or liquidity constraints, Its possible for the inputTokenAmount to not be entirely swapped. And if the amount already swapped token is greater than minimum buy amount transaction will go through. This can result in a discrepancy where the user's balance is reduced by the full amount specified (userClaim) balances[msg.sender][_token] = userStake - userClaim;, but the actual amount swapped (and thus the amount of ETH or lpETH they should receive) is less. This leads to a loss for the user, as their balance is decremented more than the amount actually converted.

The UniswapV3Feature.sellTokenForEthToUniswapV3 ultimately calls the swap function in the uniswapv3pool contract.

function swap(
        address recipient,
        bool zeroForOne,
        int256 amountSpecified,
        uint160 sqrtPriceLimitX96,
        bytes calldata data
    ) external override noDelegateCall returns (int256 amount0, int256 amount1)

amountSpecified parameter represents the intended amount of tokens to be swapped, The function attempts to use this entire specified amount (amountSpecified) for the swap operation, subject to certain conditions and constraints. There is a condition here https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/UniswapV3Pool.sol#L640 which states that The swap function executes in a loop until either, The specified amount (amountSpecified) is fully utilized for the swap or the price limit (sqrtPriceLimitX96) is reached. Thus the loop terminates when the entire amountSpecified has been used for swaps or when the price limit (sqrtPriceLimitX96) is reached. So in the case of PrelaunchPoints contract if the entire amount specified(inputTokenAmount) is not used, and amount already converted is greater than minamount to buy then we will have a situation where. The user's balance is reduced by the full amount specified (userClaim), but the actual amount swapped is less causing loss of funds to the user.

Proof of Concept

1.User initiates a claim with a specified amount to swap.

2.The swap is partially executed due to price limit reached or liquidity constraints.

3.The user's balance is reduced by the full userClaim despite a partial swap.

4.The user receives less than the deducted amount, resulting in a loss.

Tools Used

manual

Recommended Mitigation Steps

The contract should update the user's balance based on the actual amount swapped, not the amount specified(User claim).

Assessed type

Other

0xd4n1el commented 3 months ago

Technically yes, but 0x API allows you to have control of this situation. Quotes will execute as promised, so we can check that the full amount is being swapped

koolexcrypto commented 3 months ago

In addition to that,

The swap is partially executed due to price limit reached or liquidity constraints.

This is very unlikely with the current ERC20 used by the protocol.

c4-judge commented 3 months ago

koolexcrypto changed the severity to QA (Quality Assurance)