code-423n4 / 2024-05-loop-findings

4 stars 4 forks source link

Malicious users could bypass the lock process to claim any amount of lpETH #37

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

During claim process, _claim is called to claim lpETH token to user, whose amount claimedAmount is calculated based on user staked amount during lock period. However, for the scenario that user stake allowed token to claim lpETH, claimedAmount is calculated using PrelaunchPoints's balance. So malicious users could send any amount of ETH to PrelaunchPoints and call claim in a single transaction to claim any amount of lpETH they wants, which will make the whole lock process useless.

Proof of Concept

  1. Alice lock x_amount allowed Token to PrelaunchPoints by calling lock
  2. After convertAllETH and time passed startClaimDate, Alice can claim lpETH, whose amount should equals to the amount of ETH swapped by x_amount staked token. Let's define the correct amount of lpETH Alice should get is x_amt_ETH. https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L502-L504
        // Use our current buyToken balance to determine how much we've bought.
        boughtETHAmount = address(this).balance - boughtETHAmount;
        emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
  3. However, Alice send y_amount ETHs to PrelaunchPoints and call claim in a single transaction, so the actual amount lpETH she can get is y_amount + x_amt_ETH. Since this y_amount can be any number, which means that Alice could claim arbitrary amount of lpETH she wants and thusly make lock process useless. https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L261-L263
            // Convert swapped ETH to lpETH (1 to 1 conversion)
            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);

    Tools Used

    Manual Review

Recommended Mitigation Steps

use the swapped amount of ETH in _fillQuote as claimedAmount rather than address(this).balance

Assessed type

Context

c4-judge commented 4 months ago

koolexcrypto marked the issue as duplicate of #6

c4-judge commented 3 months ago

koolexcrypto marked the issue as duplicate of #33

c4-judge commented 3 months ago

koolexcrypto changed the severity to 3 (High Risk)

c4-judge commented 3 months ago

koolexcrypto marked the issue as satisfactory