function `_validateData` Allows recipient to be set to zero address when ` (_exchange == Exchange.UniswapV3)`

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/40167e469edde09969643b6808c57e25d1b9c203/src/PrelaunchPoints.sol#L439

Vulnerability details

Impact

The _validateData function which is meant to prevents fund loss by verifying swap details before execution for the recipient address to be set to the zero address when _exchange is _exchange == Exchange.UniswapV3, which could potentially allow tokens to be sent to the zero address, resulting in a loss of funds.

Proof of Concept

1. User initiates claim with UniswapV3 as the exchange option.

2. _validateData decodes the provided _data parameter.

3. _validateData allows for the recipient extracted from _data to be set to zero address because of if (recipient != address(this) && recipient != address(0)) { revert WrongRecipient(recipient); }

4. If the recipient in _data is unintentionally the zero address, tokens are sent there irretrievably during the swap.

Tools Used

manual

Recommended Mitigation Steps

    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);
            }
  +         if (recipient != address(this)) {
            revert WrongRecipient(recipient);
        } 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);
        }
    }

Assessed type

ERC20

[0xd4n1el]

We assume the data is correct so this should not happen, however this can apply as QA to make the contract clearer

[c4-judge]

koolexcrypto marked the issue as duplicate of #8

[c4-judge]

koolexcrypto changed the severity to 3 (High Risk)

[c4-judge]

koolexcrypto changed the severity to QA (Quality Assurance)

[c4-judge]

koolexcrypto marked the issue as grade-c