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

4 stars 4 forks source link

An attacker can swap all ETH in the contract to lpETH #45

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/main/src/PrelaunchPoints.sol#L259-L263 https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L491-L505

Vulnerability details

Impact

In the _claim function, due to the incorrect calculation of claimedAmount, all ETH in the contract will be swapped by the attacker to lpETH.

Proof of Concept

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

            _fillQuote(IERC20(_token), userClaim, _data);

            // Convert swapped ETH to lpETH (1 to 1 conversion)
            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);

In this code, the amount of claimAmount is all the ETH in the current contract (including the ETH locked by all users). The correct logic should be the amount after converting ERC20 to ETH - the previous amount of ETH.

In the _fillQuote function, budedETHAmount should be returned, which represents the difference between before and after eth, which is the ETH actually redeemed by the user. The actual value of ClaimedAmount should be equal to buyETHAmount, not address(this).balance.

Use an example to prove: (18 decimals)

User A locked 1ETH, address(this).balance is equal to 1eth User B locked 1ETH, address(this).balance is equal to 2eth User C locked 1WETH, address(this).balance is equal to 2eth

User C (attacker) calls prelaunchPoints.claim(WETH, 100, PrelaunchPoints.Exchange.TransformERC20, data);. At this time, the attacker has exchanged 1WETH for 1eth, address(this).balance is equal to 3eth, and claimedAmount is 3eth. , eventually the attacker will get all lpETH.

            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);

https://docs.loopfi.xyz/the-launch/the-points-program#what-happens-after-the-points-program-has-concluded

According to the document requirements, the exchange ratio between user locked tokens and lpETH is 1:1, but an attacker can get all lpETH, which I think is a high risk.

Tools Used

Manual review

Recommended Mitigation Steps

  1. Add the return value in the _fillQuote function, returns (uint256 boughtAmount)
    
    function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal returns (uint256 boughtAmount) {
2. Return the amount of boughtAmount at the end of the `_fillQuote` function
```sol
  boughtETHAmount = address(this).balance - boughtETHAmount;
  return boughtETHAmount;
  1. Fix claimedAmount in _claim function
    claimedAmount = _fillQuote(IERC20(_token), userClaim, _data);

Assessed type

Math

koolexcrypto commented 4 months ago

On claim, there is no ETH in the contract. All locked ETH were already converted to lpETH.

c4-judge commented 4 months ago

koolexcrypto marked the issue as primary issue

c4-judge commented 4 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid