search

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

4 stars 4 forks source link

Users can receive more lpETH than their locked ETH allows them #46

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/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L263

Vulnerability details

Impact

Users can lock less ERC20 and receive more lpETH, by sending ETH directly to the contract before claiming.

Proof of Concept

In the _fillQuote function the boughtETHAmount is calculated. https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L503 However in the claim function the amount that is actually deposited and given to the user is the balance of the contract: https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L262 This is because of the wrong assumption that the contract will not have any ETH balance after the convertAllETH function is called. Consider the following scenario:

  1. A user locks only a very small amount of a supported LRT.
  2. The convertAllETH function is called and now claims are unlocked.
  3. The user decides that they want more lpETH. They send ETH directly to the contract right before calling claim. Because of the wrong assumption the user will actually get more lpETH with less locked tokens, which compromises the whole purpose of the locking.

Tools Used

Manual Review

Recommended Mitigation Steps

The _fillQuote function should return the boughtETHAmount and the claimedAmount here: https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L263 should be replaced with the boughtETHAmount.

Assessed type

ETH-Transfer

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 marked the issue as satisfactory