howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

Calling the 0xAPI for an LRT is might result in TransformERC20() (0x415565b0) selector being returned, even if the exchange that is the source of the data is Uniswap_V3. This causes the _validate() data to revert with WrongSelector("0x415565b0") every time the user tries to claim/claim and stake their lpETH. The outputToken checked by manually decoding the API return data in Remix IDE with the _decodeTransformERC20() function is set to 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE - ETH, so the call to TransformERC20() should NOT be reverted. This means, that the API return data does not have to necessarily return the UNI_SELECTOR, when the source is set as Uniswap_V3.

The vulnerability is caused by the mitigation of [WP-M3] finding from LoopFi / Prelaunch / LRT #2 audit.

Proof of Concept

To better understand this problem paste the code in this gist to the PrelaunchPoints0x.test.ts test file, and run it with yarn hardhat test ./test/PrelaunchPoints0x.test.ts. It is a simplified version of the original test, where we consider only the rswETH token for clarity. Data resulting from the API call indeed begins with 0x415565b0, suggesting that we should be using the TransformERC20() function in _fillQuote. However, we never get to filling the quote, because orders[0].source is Uniswap_V3 meaning that the exchangeCode is 0. In _validateData the first branch is executed and the code fails due to WrongSelector(0x415565b0) error as explained before.

Tools Used

Manual review

Recommended Mitigation Steps

This problem has to be further investigated and only when 0x API return data is fully predictable, an appropriate mitigation can be developed. However, to simulate successful data validation the following _validateData() function implementation, where the _exchange enum parameter is ignored to allow the TransformERC20 selector- based data to be decoded ( !!! note that this is NOT a safe nor faul proof solution to this vulnerability !!!):

  function _validateData(
        address _token,
        uint256 _amount,
        Exchange _exchange,
        bytes calldata _data
    ) internal view {
        address inputToken;
        address outputToken;
        uint256 inputTokenAmount;
        address recipient;
        bytes4 selector;

        // // if exchange is uniswap _exchange == "UniswapV3" from the 0x query
        // if (_exchange == Exchange.UniswapV3) {
        //     (
        //         inputToken,
        //         outputToken,
        //         inputTokenAmount,
        //         recipient,
        //         selector
        //     ) = _decodeUniswapV3Data(_data); 
        //     if (selector != UNI_SELECTOR) {
        //         revert WrongSelector(selector);
        //     }
        //     // UniswapV3Feature.sellTokenForEthToUniswapV3(encodedPath, sellAmount, minBuyAmount, recipient) requires `encodedPath` to be a Uniswap-encoded path, where the last token is WETH, and sends the NATIVE token to `recipient`
        //     if (outputToken != address(WETH)) {
        //         revert WrongDataTokens(inputToken, outputToken);
        //     }
        // } else if (_exchange == Exchange.TransformERC20) {
        (
            inputToken,
            outputToken,
            inputTokenAmount,
            selector
        ) = _decodeTransformERC20Data(_data); // @return
        if (selector != TRANSFORM_SELECTOR) {
            revert WrongSelector(selector);
        }
        if (outputToken != ETH) {
            revert WrongDataTokens(inputToken, outputToken);
        }
        // } else {
        //     revert WrongExchange();
        // }

        if (inputToken != _token) {
            revert WrongDataTokens(inputToken, outputToken);
        }
        if (inputTokenAmount != _amount) {
            revert WrongDataAmount(inputTokenAmount);
        }
        if (recipient != address(this) && recipient != address(0)) {
            revert WrongRecipient(recipient);
        }
    }

This shows, that the return data is indeed correct, the issue lays within the assumption [source=uniswapv3 implies selector=UNI_SELECTOR].

Assessed type

Invalid Validation

0xd4n1el commented 4 months ago

This is indeed a problem on the test implementations, however TransformERC20 always give ETH as an outputToken and Uniswav_V3 always give WETH as outputToken so the validations in the contract are correct and not a vulnerability in the contract itself

c4-judge commented 3 months ago

koolexcrypto marked the issue as primary issue