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

0 stars 0 forks source link

User can frontrun claiming and steal all of the locked ethers for himself #391

Open c4-bot-2 opened 5 months ago

c4-bot-2 commented 5 months ago

Lines of code

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

Vulnerability details

Description

According to the NatSpec comment of the receive() function: "ETH sent to this contract directly will be locked forever." - all ethers sent directly to the contract should be locked forever. However, ETH sent directly to the contract won't be locked forever but will be distributed among users who have locked ETH. The logic for distribution is insufficient though. A malicious user can frontrun the first user who tries to claim his tokens. The attacker claims a ERC20 token and this way the he will receive all of the locked ethers to himself.

        } 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;
            // @audit transfers the whole balance of the contract to the user
            ❌ lpETH.deposit{value: claimedAmount}(_receiver);
        }

Link

Impact

Malicious user can steal all other users' locked ethers.

Proof of Concept

Tools Used

Manual Review

Recommended Mitigation Steps

Calculate the amount of shares the user should receive instead of transferring the whole balance of the contract to the user.

Assessed type

ETH-Transfer

0xSorryNotSorry commented 5 months ago

Technically valid, and it doesn´t pose a risk due to sending ETH to a contract is shooting own foot.

0xSorryNotSorry commented 5 months ago

@howlbot reject

radeveth commented 4 months ago

Hey @koolexcrypto,

This finding was misjudged and missed to be moved to the finding repo.

It is the same as:

Please move it and make it a valid duplicate of https://github.com/code-423n4/2024-05-loop-findings/issues/33.

koolexcrypto commented 4 months ago

Hi @radeveth

This should be moved to the main repo since it is a valid issue.

0xSorryNotSorry commented 4 months ago

@howlbot undo reject

howlbot-integration[bot] commented 4 months ago

sorry @0xSorryNotSorry, that command failed

0xSorryNotSorry commented 4 months ago

@howlbot accept