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

4 stars 4 forks source link

The locked ETH balance can be used in the `_claim` function #54

Closed howlbot-integration[bot] closed 3 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#L262

Vulnerability details

Impact

If someone accidentally sends ETH directly to the PrelaunchPoints contract, a user could potentially detect this and call the _claim function to gain more tokens than they originally provided token for. Essentially, they could claim tokens for more than they deposited.

Proof of Concept

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

Tools Used

Manual review

Recommended Mitigation Steps

To prevent this, it's recommended to modify the _fillQuote function to return the actual amount of ETH. This claimed amount should then be used to deposit.

https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L491~L505

-    function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal {
+    function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal returns (uint256 boughtETHAmount) {
        // Track our balance of the buyToken to determine how much we've bought.
-       uint256 boughtETHAmount = address(this).balance;
+        boughtETHAmount = address(this).balance;

        require(_sellToken.approve(exchangeProxy, _amount));

        (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
        if (!success) {
            revert SwapCallFailed();
        }

        // Use our current buyToken balance to determine how much we've bought.
        boughtETHAmount = address(this).balance - boughtETHAmount;
        emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
    }
        // At this point there should not be any ETH in the contract
        // Swap token to ETH
-        claimedAmount = _fillQuote(IERC20(_token), userClaim, _data);
+        claimedAmount = _fillQuote(IERC20(_token), userClaim, _data);

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

Assessed type

ETH-Transfer

c4-judge commented 4 months ago

koolexcrypto marked the issue as duplicate of #18

c4-judge commented 3 months ago

koolexcrypto changed the severity to 3 (High Risk)

c4-judge commented 3 months ago

koolexcrypto changed the severity to 2 (Med Risk)

c4-judge commented 3 months ago

koolexcrypto marked the issue as partial-50

c4-judge commented 3 months ago

koolexcrypto changed the severity to 3 (High Risk)

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 not a duplicate

c4-judge commented 3 months ago

koolexcrypto changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

koolexcrypto marked the issue as grade-c