Users can claim and mint tokens that they have not locked

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

A user that has locked just 1 wei of the allowed tokens can claim an arbitrary amount of the lpETH token.

Proof of Concept

This is the _claim() function:

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

If a user has locked just 1 wei of any of the allowed tokens, then he can do the following:

1. Directly send Ether to the contract for the amount he wishes to claim
2. Call the claim() function with the token he has locked for 1 wei
3. He will end up in the else statement as his token is not ETH
4. His 1 wei of the token gets swapped to ETH
5. Then, the claimedAmount variable is equal to address(this).balance which he just increased as he sent Ether directly to the contract

While the implementation of the lpETH contract is unclear, such issue can definitely cause a lot of unexpected issues depending on the implementation of lpETH and other contracts. Furthermore, that makes the event emission wrong and by the contest page in Code4rena, we can see that they are tracking different events on the backend, potentially causing other issues.

Tools Used

Manual Review

Recommended Mitigation Steps

Change the _fillQuote() function to return the boughtETHAmount variable and use it as the claimedAmount instead.

Assessed type

Other

[c4-judge]

koolexcrypto marked the issue as duplicate of #6

[c4-judge]

koolexcrypto marked the issue as duplicate of #33

[c4-judge]

koolexcrypto marked the issue as partial-75

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[cholakovvv]

@koolexcrypto why this issue is marked as partial?

[koolexcrypto]

Hi @cholakovvv

All issues that don't mention bypassing the locking duration will have partial credits. still evaluating the percentage

[cholakovvv]

@koolexcrypto I believe this report clearly mentions that the user can deposit 1 wei and then call claim(). Since that function can only be called after the start claim date, it is clear what the person behind the report had in mind.

[koolexcrypto]

The issue describes a scenario. but didn't clearly mentioned the impact of bypassing locking duration. This got 75% credit unlike others which will get less.

such issue can definitely cause a lot of unexpected issues depending on the implementation of lpETH and other contracts. Furthermore, that makes the event emission wrong and by the contest page in Code4rena, we can see that they are tracking different events on the backend, potentially causing other issues.