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

4 stars 4 forks source link

Loss of funds for users who claim vested `lpETH` by swapping via `UniswapV3` #74

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/main/src/PrelaunchPoints.sol#L192 https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L252-L263 https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L491-L505 https://github.com/code-423n4/2024-05-loop/blob/main/src/PrelaunchPoints.sol#L405

Vulnerability details

Impact

Users who lock wrapped LRTs can claim their vested lpETH by swapping their tokens via UniswapV3 or TransformERC20.

If they use UniswapV3, their token must be swapped for WETH. The problem is that the WETH received from the swap is never unwrapped to ETH, so when the protocol attempts to send lpETH to the user, it will convert ETH to lpETH by a 1:1 conversion ratio, but since the WETH was never unwrapped to ETH, that amount will be 0.

In addition WETH is not recoverable because it is always in the isTokenAllowed list, which causes recoverERC20() to revert. Therefore users will lose all funds and their WETH may be locked in the contract forever.

Proof of Concept

The PrelaunchPoints contract allows users to lock ETH, WETH, and wrapped LRTs.

    function _processLock(address _token, uint256 _amount, address _receiver, bytes32 _referral)
        internal
        onlyBeforeDate(loopActivation)
    {
        if (_amount == 0) {
            revert CannotLockZero();
        }

@>      if (_token == ETH) {
            totalSupply = totalSupply + _amount;
            balances[_receiver][ETH] += _amount;
        } else {
@>          if (!isTokenAllowed[_token]) {
                revert TokenNotAllowed();
            }
            IERC20(_token).safeTransferFrom(msg.sender, address(this), _amount);

@>          if (_token == address(WETH)) {
                WETH.withdraw(_amount);
                totalSupply = totalSupply + _amount;
                balances[_receiver][ETH] += _amount;
            // @audit if token is not WETH or ETH, but is in the `isTokenAllowed` list, it is accepted
 @>         } else {
                balances[_receiver][_token] += _amount;
            }
        }

        emit Locked(_receiver, _amount, _token, _referral);
    }

Once the owner calls convertAllETH(), all the ETH that was locked by users is converted to lpETH

    function convertAllETH() external onlyAuthorized onlyBeforeDate(startClaimDate) {
        if (block.timestamp - loopActivation <= TIMELOCK) {
            revert LoopNotActivated();
        }

        // deposits all the ETH to lpETH contract. Receives lpETH back
        uint256 totalBalance = address(this).balance;
        lpETH.deposit{value: totalBalance}(address(this));

        totalLpETH = lpETH.balanceOf(address(this));

        // Claims of lpETH can start immediately after conversion.
        startClaimDate = uint32(block.timestamp);

        emit Converted(totalBalance, totalLpETH);
    }

Users can claim their lpETH by calling claim() or claimAndStake(), which call the internal _claim():

    function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        internal
        returns (uint256 claimedAmount)
    {
        uint256 userStake = balances[msg.sender][_token];
        if (userStake == 0) {
            revert NothingToClaim();
        }
        if (_token == ETH) {
            claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
            balances[msg.sender][_token] = 0;
            lpETH.safeTransfer(_receiver, claimedAmount);

        // @audit since they did not lock ETH or WETH, this branch will be executed
@>      } 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
@>          _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);
    }

Let's follow the flow on what happens when _token is not ETH. A call to _validateData is made:

    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);
            }
            // @audit output token must be WETH
@>          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);
            }
        } 

        .
        .
        .
    }

To decide how much lpETH to give to the user, the tokens they locked must be converted to ETH. This is done by swapping via UniswapV3 or TransformERC20.

If the user decides to swap via UniswapV3, the user must swap their tokens for WETH, otherwise _validateData will revert.

Once the selectors are validated, the user's balances mapping is updated, and a call to _fillQuote() is made

    function _fillQuote(IERC20 _sellToken, uint256 _amount, bytes calldata _swapCallData) internal {
        // Track our balance of the buyToken to determine how much we've bought.
@>      uint256 boughtETHAmount = address(this).balance;

        require(_sellToken.approve(exchangeProxy, _amount));

@>      (bool success,) = payable(exchangeProxy).call{value: 0}(_swapCallData);
        if (!success) {
            revert SwapCallFailed();
        }

        // Use our current buyToken balance to determine how much we've bought.
@>      boughtETHAmount = address(this).balance - boughtETHAmount;
        emit SwappedTokens(address(_sellToken), _amount, boughtETHAmount);
    }

Here the swap is executed and the ETH before and after the swap is calculated. However, recall that the swap via UniswapV3 is for WETH, not for ETH. Therefore the amount of ETH in the contract will still be 0, since that WETH was never unwrapped to ETH.

When the following lines are executed after the call to _fillQuote():

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

claimedAmount = 0, since the contract has no ETH. That amount of lpETH will be sent to the user.

For the amount of tokens swapped via UniswapV3, the user received 0 lpETH, and loses all funds that were used for the swap.

To make matters worse, the WETH received from the swap is not recoverable:

    constructor(address _exchangeProxy, address _wethAddress, address[] memory _allowedTokens) {
        owner = msg.sender;
        exchangeProxy = _exchangeProxy;
@>      WETH = IWETH(_wethAddress);

        loopActivation = uint32(block.timestamp + 120 days);
        startClaimDate = 4294967295;

        uint256 length = _allowedTokens.length;
        for (uint256 i = 0; i < length;) {
            isTokenAllowed[_allowedTokens[i]] = true;
            unchecked {
                i++;
            }
        }
        // @audit WETH is always on the allowed list
@>      isTokenAllowed[_wethAddress] = true;
    }
    function recoverERC20(address tokenAddress, uint256 tokenAmount) external onlyAuthorized {
        if (tokenAddress == address(lpETH) || isTokenAllowed[tokenAddress]) {
@>          revert NotValidToken();
        }
        IERC20(tokenAddress).safeTransfer(owner, tokenAmount);

        emit Recovered(tokenAddress, tokenAmount);
    }

WETH is always in the isTokenAllowed list, causing the call to recoverERC20() to revert. The WETH is non-recoverable and user funds are lost.

Tools Used

Manual Review.

Recommended Mitigation Steps

Consider making the following changes to convert the WETH received from the UniswapV3 swap to ETH.

Utilize the exchange parameter to determine if WETH is expected and proceed to unwrap:

    function _claim(address _token, address _receiver, uint8 _percentage, Exchange _exchange, bytes calldata _data)
        internal
        returns (uint256 claimedAmount)
    {
        uint256 userStake = balances[msg.sender][_token];
        if (userStake == 0) {
            revert NothingToClaim();
        }
        if (_token == ETH) {
            claimedAmount = userStake.mulDiv(totalLpETH, totalSupply);
            balances[msg.sender][_token] = 0;
            lpETH.safeTransfer(_receiver, claimedAmount);
        } 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
-           _fillQuote(IERC20(_token), userClaim, _data);

+           if(_exchange == Exchange.UniswapV3){
+               uint256 wethBeforeSwap = IERC20(address(WETH)).balanceOf(address(this));
+               _fillQuote(IERC20(_token), userClaim, _data);
+               uint256 wethReceived = IERC20(address(WETH)).balanceOf(address(this)) - wethBeforeSwap;
+               WETH.withdraw(wethReceived);
+           }
+           else{
+               _fillQuote(IERC20(_token), userClaim, _data);
+           }

            // Convert swapped ETH to lpETH (1 to 1 conversion)
            claimedAmount = address(this).balance;
            lpETH.deposit{value: claimedAmount}(_receiver); //@audit looks like they didnt send it to caller
        }
        emit Claimed(msg.sender, _token, claimedAmount);
    }

Assessed type

Token-Transfer

0xd4n1el commented 3 months ago

Uniswap v3 calls to 0x automatically unwrap to ETH once the call is done with outputToken=ETH. However in the path this is encoded as WETH

c4-judge commented 3 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid