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

4 stars 4 forks source link

`claim()` and `claimAndStake()` Will Not Work with UniswapV3 as the Exchange Parameter #108

Closed howlbot-integration[bot] closed 4 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#L204-L235 https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L414

Vulnerability details

Bug Description

Users can call claim() to receive their vested lpETH, or claimAndStake() to additionally stake them in a Loop vault for extra rewards.

PrelaunchPoints.sol#L204-L235

    /**
     * @dev Called by a user to get their vested lpETH
     * @param _token      Address of the token to convert to lpETH
     * @param _percentage Proportion in % of tokens to withdraw. NOT useful for ETH
     * @param _exchange   Exchange identifier where the swap takes place
     * @param _data       Swap data obtained from 0x API
     */
    function claim(address _token, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        external
        onlyAfterDate(startClaimDate)
    {
        _claim(_token, msg.sender, _percentage, _exchange, _data);
    }

    /**
     * @dev Called by a user to get their vested lpETH and stake them in a
     *      Loop vault for extra rewards
     * @param _token      Address of the token to convert to lpETH
     * @param _percentage Proportion in % of tokens to withdraw. NOT useful for ETH
     * @param _exchange   Exchange identifier where the swap takes place
     * @param _data       Swap data obtained from 0x API
     */
    function claimAndStake(address _token, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        external
        onlyAfterDate(startClaimDate)
    {
        uint256 claimedAmount = _claim(_token, address(this), _percentage, _exchange, _data);
        lpETH.approve(address(lpETHVault), claimedAmount);
        lpETHVault.stake(claimedAmount, msg.sender);

        emit StakedVault(msg.sender, claimedAmount);
    }

Users can call these functions to convert their deposited LRTs into ETH, which is then deposited into the lpETH contract to receive vested lpETH tokens.

When users choose to convert their deposited LRT tokens to lpETH, they can opt to swap through either UniswapV3 or TransformERC20.

PrelaunchPoints.sol#L37-L40

    enum Exchange {
        UniswapV3,
        TransformERC20
    }

However, choosing to swap through UniswapV3 will always fail because the data obtained from the 0x API always returns the TRANSFORM_SELECTOR = 0x415565b0, which leads to a revert when this check fails:

PrelaunchPoints.sol#L414C17-L414C17

    /**
     * @notice Validates the data sent from 0x API to match desired behaviour
     * @param _token     address of the token to sell
     * @param _amount    amount of token to sell
     * @param _exchange  exchange identifier where the swap takes place
     * @param _data      swap data from 0x API
     */
    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 == 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);
            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 is the command to make a Swap Quote request to 0x (add your API key):

curl --location --request GET 'https://api.0x.org/swap/v1/quote?sellToken=0xeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee&buyToken=0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48&sellAmount=1000000000000000000' --header '0x-api-key: [api-key]'

but the data returned by the 0x API always starts by 0x415565b0.

Recommendation

Remove the UNI_SELECTOR check, as the correct checks are already implemented in exchangeProxy:

ZeroEx#L48

    /// @dev Forwards calls to the appropriate implementation contract.
    fallback() external payable {
        bytes4 selector = msg.data.readBytes4(0);
        address impl = getFunctionImplementation(selector);
        if (impl == address(0)) {
            _revertWithData(LibProxyRichErrors.NotImplementedError(selector));
        }

PrelaunchPoints.sol#L412-L416

        if (_exchange == Exchange.UniswapV3) {
            (inputToken, outputToken, inputTokenAmount, recipient, selector) = _decodeUniswapV3Data(_data);
-           if (selector != UNI_SELECTOR) {
-               revert WrongSelector(selector);
-           }

Assessed type

Other

koolexcrypto commented 4 months ago

This is out of scope, since it is on the frontend. However, still invalid as you should pass includedSources in the request. The default is 0x, that's why you receive the TRANSFORM_SELECTOR => 0x415565b0

c4-judge commented 4 months ago

koolexcrypto marked the issue as primary issue

c4-judge commented 4 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid