User can lock low amount worth of any token different than WETH and get as much lpETH as he wants.
Proof of Concept
The user will lock via lock() function, then after some time when the owner calls the convertAllETH() function and everybody is able to claim or stake their lpETH, the user will send ether to the contract and immediately call claim() or claimAndStake() functions. No matter which function he will call, because they are both leading to the following block of code in the _claim() function:
} 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);
}
emit Claimed(msg.sender, _token, claimedAmount);
It deposits the ETH balance of the contract to the receiver, assuming that there is no ETH left in the contract after the convertAllETH() function is called. This sabotages the whole idea of the locking mechanism, making it easy for users to trick the system
Tools Used
Manual review
Recommended Mitigation
return the boughtETHAmount from _fillQuote function and deposit it, instead of depositing the ETH balance of the contract
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#L492-L505
Vulnerability details
Impact
User can lock low amount worth of any token different than
WETH
and get as muchlpETH
as he wants.Proof of Concept
The user will lock via
lock()
function, then after some time when the owner calls theconvertAllETH()
function and everybody is able to claim or stake theirlpETH
, the user will send ether to the contract and immediately callclaim()
orclaimAndStake()
functions. No matter which function he will call, because they are both leading to the following block of code in the_claim()
function:It deposits the ETH balance of the contract to the receiver, assuming that there is no ETH left in the contract after the
convertAllETH()
function is called. This sabotages the whole idea of the locking mechanism, making it easy for users to trick the systemTools Used
Manual review
Recommended Mitigation
return the
boughtETHAmount
from_fillQuote
function and deposit it, instead of depositing the ETH balance of the contractAssessed type
Other