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

4 stars 4 forks source link

Malicious actors could exploit `claim()` function to get lpETH at a discount #42

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

Vulnerability details

Impact

Malicious actors will receive more lpETH than they should relative to the value of the LRT they locked in the contract, ultimately resulting in theft from the protocol and other users who are utilizing it.

Proof of Concept

If users deposited LRTs instead of ETH, obtaining the corresponding lpETH amount against their LRTs is executed as follows, as stated on the contest page: "The conversion for LRTs happens on each claim by using 0x API. This is triggered by each user." However, while claiming lpETH against the LRT value they've deposited, malicious actors could manipulate the protocol to receive more lpETH than they are entitled to.

This issue arises from the expectation of the protocol team, that when users claim their LRTs, the contract balance would not contain any ETH. Thus, the claimedAmount, retrieved as claimedAmount = address(this).balance;, where address(this).balance is expected to reflect the value of LRT in terms of ETH from the 0x API. Based on claimedAmount also is determined the amount of lpETH that the user should receive. However, a malicious actor could exploit this by transferring ETH to the contract before calling the claim() function. This vulnerability is profitable if the ETH price is lower than the LRT price in ETH terms. For example, at the time of writing this report, the price of UNIETH is $3,340.88, and the ETH price is $3,132.95. Therefore, this exploitation would allow the malicious actor to receive more lpETH at a cheaper price relative to the LRT.

    function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        internal
        returns (uint256 claimedAmount)
    {
        uint256 userStake = balances[msg.sender][_token];
        ...
         else {
            uint256 userClaim = userStake * _percentage / 100;
            _validateData(_token, userClaim, _exchange, _data);
            balances[msg.sender][_token] = userStake - userClaim;

@>          // At this point there should not be any ETH in the contract
            // Swap token to ETH
            // @audit-issue This can be not the case, as there is receive() function and malicious actor can send ETH to the contract
            _fillQuote(IERC20(_token), userClaim, _data);

            // Convert swapped ETH to lpETH (1 to 1 conversion)
@>          claimedAmount = address(this).balance;
@>          lpETH.deposit{value: claimedAmount}(_receiver);
        }
        emit Claimed(msg.sender, _token, claimedAmount);
    }

Tools Used

VSCode

Recommended Mitigation Steps

The amount to be deposited in lpETH should be calculated as follows:

    function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        internal
        returns (uint256 claimedAmount)
    {
        ...
        } else {
                ...

            uint256 contractBalanceBefore0x = address(this).balance;
            _fillQuote(IERC20(_token), userClaim, _data);

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

Implementing this approach ensures that if there are direct transfers to the contract, the amount received from 0x matches exactly the value of the LRT.

Assessed type

ETH-Transfer

c4-judge commented 4 months ago

koolexcrypto marked the issue as primary issue

0xd4n1el commented 4 months ago

lpETH is a wrapped ETH contract, so in this case user does not get a discount in any way

c4-judge commented 3 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid