LoopFi - bbl4de's results

Lines of code

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

Vulnerability details

Impact

When claimedAmount is calculated in the (_token == ETH) branch, the amount of lpETH received may be greater than the staked ETH in case there was an accidental direct deposit of ETH to the contract. The user making this transfer of ETH rightfully losses it without a way to recover, the issue is that the ETH can be considered to be lost as much as it would be when sent to a dummy address, as there is no way to recover it from the contract - you can only recover ERC20s or use the withdraw() function as a user. The excess balance causes totalLpETH to be greater than totalSupply because of the following lines in convertAllETH:

uint256 totalBalance = address(this).balance;
lpETH.deposit{value: totalBalance}(address(this));
totalLpETH = lpETH.balanceOf(address(this));

Therefore, it increases the claimedAmount for each user. This is causing the conversion rate between ETH and lpETH to not equal 1. Moreover, considering the (_token != ETH) branch, a user can _fillQuote with amount equal to userClaim, however the claimedAmount can be increased by simply sending ETH directly to the contract right before the call to claim() or claimAndStake() functions. Again, this makes the conversion to change from 1:1, because ETH sent directly to the contract is NOT accessible by anyone including the owner.

Both of these situations make the lpETH inflationary as users essentially get more lpETH for the same amount of real stake if they do what's explained above for both _claim() function token branches.

Proof of Concept

To run a PoC proving how the received lpETH is indeed increased when the user sends ETH directly to the contract right before calling claim() change the code in PrelaunchPoints0x.test.ts to the code in this gist and run the test with yarn hardhat test ./test/PrelaunchPoints0x.test.ts.

Tools Used

Manual Review, Hardhat

Recommended Mitigation Steps

To mitigate this issue consider changing the approach from push to pull, meaning that in convertAllETH the owner does not deposit all ETH to the lpETH contract. Thanks to this, totalSupply and totalLpETH variables can be removed and the user gets the appropriate amount of lpETH based on their share of the contract's balance.

Assessed type

Token-Transfer