Claiming with `UniswapV3` as the Exchange Parameter and `msg.sender` as the Recipient Won't Deposit into lpETH, and the Wrong Event Will Be Emitted

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L258-L265 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L439-L441

Vulnerability details

Bug Description

When calling claim() or claimAndStake() to convert their deposited LRT tokens to ETH and then to lpETH, users can choose between two exchanges for the swap-UniswapV3 and TransformERC20, and two recipients-the PrelaunchPoints contract or themselves (msg.sender).

    enum Exchange {
        UniswapV3,
        TransformERC20
    }

        //@audit "The recipient of the bought tokens. Can be zero for sender."
        if (recipient != address(this) && recipient != address(0)) { 
            revert WrongRecipient(recipient);
        }

Once LRTs are swapped to ETH, they are supposed to be deposited into lpETH to credit the _receiver (user) with the lpETH tokens owed.

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

However, when choosing to use claim() or claimAndStake() with UniswapV3 as the exchange parameter and address(0) (msg.sender) as the recipient, this line will return 0 and won't account for the effective claimed amount: claimedAmount = address(this).balance; Consequently, it will not reflect the actual amount claimed, and no tokens will be deposited into lpETH to be credited to the user.

Furthermore, the Claimed event will erroneously emit claimedAmount == 0, which can disrupt off-chain accounting performed by the LoopFi backend.

Recommendation

To accurately track the claimed amount, use the return value uint256 buyAmount from UniswapV3Feature.sellTokenForEthToUniswapV3() instead of relying on claimedAmount = address(this).balance;.

Assessed type

Other

[c4-judge]

koolexcrypto changed the severity to QA (Quality Assurance)

[0xd4n1el]

This is technically possible, but very unlikely. If users decide to set recipient to address(0), their tokens can be lost (need a PoC). We are changing the recipient validation

[c4-judge]

koolexcrypto marked the issue as grade-c