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

4 stars 4 forks source link

Race condition on `PrelaunchPoints::_claim()` allow malicious actor deposit all other user `ETH` to `lpETHVault` to get `lpETH` more than they should be #49

Closed howlbot-integration[bot] closed 5 months ago

howlbot-integration[bot] commented 5 months ago

Lines of code

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

Vulnerability details

Impact

User loses all ETH balance and cannot claim lpETH

Proof of Concept

PrelaunchPoints::_claim() has a function for users to claim lpETH based on the amount of ETH or other LRT's tokens that are locked. When a user only claims lpETH using the ETH they own, this does not become an issue but the issue lies when the user wants to claim lpETH using LRT's which have previously been locked.

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

When a user wants to claim lpETH with the LRT's she/he has then the steps are:

  1. User calls the claim function and enters the LRT's address
  2. Users can also set how many LRT's will be used to claim lpETH by entering a percentage value
  3. After all variables have been entered and are valid, there are two main steps that must be taken
  4. The protocol validates the data that the user enters with the _validateData() function
  5. After the data is validated, the LRT's token will be swapped for ETH according to the percentage entered by the user using the _fillQuote() function
  6. ETH proceeds from swapping LRT's tokens will be sent to the PrelaunchPoints contract
  7. To claim lpETH, then ETH is deposited with the lpETH.deposit function and the user gets lpETH according to the amount of the ETH balance in PrelaunchPoints

In Ethereum, all transactions appear on the network before being accepted. Users can see upcoming token claim. As a result, an attacker can front-run large token claim.

Exploit Scenario

  1. Alice has 100 LRT token and she call claim function to get lpETH
  2. Bob monitors transactions in the PrelaunchPoints contract and waits for the swap (LRT to ETH) to be successful
  3. Then Bob calls the claim function with as few LRT amount as possible (i.e 1 LRT) and with higher gas price, Bob's transaction will be executed first
  4. Alice transactions will revert because the ETH balance in the PrelaunchPoints contract has been deposited through a transaction made by Bob, that way Alice loses her LRT assets and doesn't get any lpETH and Bob with the little LRT capital he has gets the amount of lpETH that Alice should have

Tools Used

Manual review

Recommended Mitigation Steps

Consider adding balance check

        } else {
            uint256 userClaim = userStake * _percentage / 100;
            _validateData(_token, userClaim, _exchange, _data);
            balances[msg.sender][_token] = userStake - userClaim;

        balanceBefore = address(this).balance;
            // At this point there should not be any ETH in the contract
            // Swap token to ETH
            _fillQuote(IERC20(_token), userClaim, _data);
            balanceAfter = address(this).balance;

            // Convert swapped ETH to lpETH (1 to 1 conversion)
            claimedAmount = balanceAfter - balanceBefore;
            if (claimedAmount != userClaim) {
                revert BalanceNotMatch;}
            lpETH.deposit{value: claimedAmount}(_receiver);
        }

Assessed type

Other

c4-judge commented 5 months ago

koolexcrypto marked the issue as primary issue

c4-judge commented 5 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid