Excess ETH (Protocol Fee Refund) Not Returned to User in _fillQuote Function

[howlbot-integration[bot]]

Lines of code

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

Vulnerability details

Impact

When a user claims tokens other than ETH, the _claim function calls the _fillQuote function to swap the tokens for ETH using the 0x exchange proxy. If there is any excess ETH (protocol fee refund) sent back from the 0x exchange proxy after the swap, it is not refunded to the user who initiated the claim. Instead, the excess ETH is converted to lpETH and sent to the _receiver, which is not the intended behavior. This results in the user losing the protocol fee refund they are entitled to.

Proof of Concept

The issue occurs in the _fillQuote function. Here's a detailed explanation:

In the _claim function, when a user claims tokens other than ETH, it calls the _fillQuote function to swap the tokens for ETH:

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

// PrelaunchPoints.sol
259:            _fillQuote(IERC20(_token), userClaim, _data);

After the swap, the _claim function converts the swapped ETH to lpETH and sends it to the _receiver:

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

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

Now, let's look at the _fillQuote function: https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L491-L505

491:    function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal {
492:        // Track our balance of the buyToken to determine how much we've bought.
493:        uint256 boughtETHAmount = address(this).balance;
494: 
495:        require(_sellToken.approve(exchangeProxy, _amount));
496: 
497:        (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
498:        if (!success) {
499:            revert SwapCallFailed();
500:        }
501: 
502:        // Use our current buyToken balance to determine how much we've bought.
503:        boughtETHAmount = address(this).balance - boughtETHAmount;
504:        emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
505:    }

The _fillQuote function calculates the amount of ETH bought (boughtETHAmount) by taking the difference between the contract's balance before and after the swap. However, it does not account for any excess ETH that might be sent back from the 0x exchange proxy as a protocol fee refund.

As a result, any excess ETH remains in the contract and is not refunded to the user who initiated the claim (msg.sender). Instead, the excess ETH is converted to lpETH along with the bought ETH and sent to the _receiver in the _claim function.

Scenario: Let's say Bob initiates a claim for 100 tokens (not ETH) using the claim function. The _claim function calls _fillQuote to swap these tokens for ETH. After the swap, the 0x exchange proxy sends back the bought ETH along with a 0.1 ETH protocol fee refund. However, the _fillQuote function does not handle the refund, and the 0.1 ETH remains in the contract. The _claim function then converts the entire contract balance (bought ETH + 0.1 ETH refund) to lpETH and sends it to the _receiver. As a result, Bob loses the 0.1 ETH refund he was supposed to receive.

Tools Used

Manual review

Recommended Mitigation Steps

The _fillQuote function should be modified to calculate the refund amount after the swap and send it back to msg.sender before the bought ETH is converted to lpETH in the _claim function. Here's a recommended change:

function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal {
-    uint256 boughtETHAmount = address(this).balance;
+    uint256 initialBalance = address(this).balance;

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

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

-     boughtETHAmount = address(this).balance - boughtETHAmount;
+    uint256 boughtETHAmount = address(this).balance - initialBalance;

    // Refund excess ETH (protocol fee) to the user who initiated the claim
+    uint256 refundAmount = address(this).balance - boughtETHAmount;
+    if (refundAmount > 0) {
+        payable(msg.sender).transfer(refundAmount);
+    }

    emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
}

With this changes, any excess ETH (protocol fee refund) will be sent back to msg.sender before the bought ETH is converted to lpETH in the _claim function. This makes sure that the user who initiated the claim receives the refund they are entitled to.

Assessed type

Token-Transfer

[c4-judge]

koolexcrypto marked the issue as primary issue

[0xd4n1el]

The user that initiates the transaction is entitled to all the ETH in the 0x swap including fee refunds, since the user is the one in posession of the balance of LRT locked in the contract. Also the mitigation is incorrect

[c4-judge]

koolexcrypto marked the issue as unsatisfactory: Invalid