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

4 stars 4 forks source link

Any user can bypass the conversion of LRTs to lpETH during the claim process by deploying custom pools on Uniswap #73

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#L497

Vulnerability details

Impact

Users can skip the conversion of their staked in PrelaunchPoints LRTs without losing points by deploying a mock ERC20 token and two pools on Uniswap, which will negatively affect the main purpose of PrelaunchPoints - commitment to the conversion. Bypassing the conversion is intended to only be allowed in withdraw().

There's a mention of a similar issue in the Publicly Known Issues section:

Crafting malicious calldata so that users get less funds as expected when claiming lpETH on LRT deposits (e.g. by setting big slippage/price impact)

But it does not apply here because its purpose is to invalidate user mistakes, social engineering attacks, and standard swap frontrunning (which are invalid anyway by the C4 rules), and the current report is about a malicious calldata that benefits the user.

Proof of Concept

During the claiming process, users are allowed to use their own calldata for the 0x call with some restrictions (see _validateData()). They can also choose the function that will execute the calldata: transformERC20() or sellTokenForEthToUniswapV3().

In both cases, there's no validation for any intermediary swaps or slippage, which allows users to use self-created pools to extract their deposit through the slippage and skip the conversion to lpETH.

Example with sellTokenForEthToUniswapV3():

https://github.com/0xProject/protocol/blob/e66307ba319e8c3e2a456767403298b576abc85e/contracts/zero-ex/contracts/src/features/UniswapV3Feature.sol#L107-L126

    function sellTokenForEthToUniswapV3(
        bytes memory encodedPath,
        uint256 sellAmount,
        uint256 minBuyAmount,
        address payable recipient
    ) public override returns (uint256 buyAmount) {
        buyAmount = _swap(
            encodedPath,
            sellAmount,
            minBuyAmount,
            msg.sender,
            address(this) // we are recipient because we need to unwrap WETH
        );
        WETH.withdraw(buyAmount);
        // Transfer ETH to recipient.
        (bool success, bytes memory revertData) = _normalizeRecipient(recipient).call{value: buyAmount}("");
        if (!success) {
            revertData.rrevert();
        }
    }
  1. Deploy a mock ERC20 token (MOCK)
  2. Deploy two pools on Uniswap:
    1. LRT/MOCK with a larger amount of LRT and some negligible amount of MOCK (e.g., 10000000:1)
    2. MOCK/WETH with a larger amount of MOCK and some negligible amount of WETH
  3. Craft a calldata with the encodedPath containing the required LRT as the input token, MOCK as an intermediary token, WETH as the output token, and minBuyAmount set to 0.
  4. Call claim(). 0x will swap the LRT to MOCK through the first pool with the slippage about 99%, resulting in some small amount of MOCK; then it'll swap MOCK to WETH through the second pool, resulting in some dust amount of WETH and finishing the transaction.
  5. Remove the liquidity from the first pool, which will contain the deposit.

Tools Used

Manual review

Recommended Mitigation Steps

Review the validation of the calldata. Consider using only predefined routes.

Assessed type

Uniswap

0xd4n1el commented 4 months ago

Validating any possible path is unfeasible for gas limits. By using Claimed event we track if user indeed withdraw and adjust points accordingly. In this case the contract does not need further modifications

c4-judge commented 4 months ago

koolexcrypto marked the issue as primary issue

koolexcrypto commented 3 months ago

As stated by the sponsor, Claimed event is used which is emitted upon claim, claimedAmount is address(this).balance. So, the user won't receive more points.

            // Convert swapped ETH to lpETH (1 to 1 conversion)
            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver);
        }
        emit Claimed(msg.sender, _token, claimedAmount);
c4-judge commented 3 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid