WithdrawBatch.sol - if the withdraw token specified is ETH, it isn't correctly sent to the user

[hats-bug-reporter[bot]]

Github username: @PlamenTSV Twitter username: @p_tsanev Submission hash (on-chain): 0xe56b35d7e401ee407d1800877632f12a2c45c2a1c495ab7ec0b4f4fd968e382d Severity: medium

Description: Description\ WithdrawBatch.sol contains a singular function multiTokenSwapAndWithdraw() meant to ease up user experience and allow them to interact with the system in batches and by using ETH. The function takes a specified withdraw token, withdraws the tokens from the portfolio and swaps them into the withdraw token, which is sent to the caller. This way he can claim all his collateral in the form of 1 token of his choosing. The system supposedly allows ether to be withdrawn as well, however that case is not handled.

Attack Scenario\ Inside _getTokenBalanceOfUser we handle the case in which the user wants to withdraw his tokens into ETH

function _getTokenBalanceOfUser(
    address _token,
    address _user
  ) internal view returns (uint256 balance) {
    if (_token == ETH_ADDRESS) {
      balance = _user.balance;
    } else {
      balance = _getTokenBalance(_token, _user);
    }
  }

to keep track of the received balance before and after. However, the loop responsible for converting and sending his funds is flawed, as the else block:

else {
        (bool success, ) = SWAP_TARGET.delegatecall(_callData[i]);
        if (!success) revert ErrorLibrary.CallFailed();

        // Return any leftover dust to the user
        TransferHelper.safeTransfer(
          _token,
          user,
          _getTokenBalance(_token, address(this))
        );
      }

attempts to do an ERC20 transfer of the swapped token. Suppose we set the withdraw token to ETH (the ETH address variable) The loop would go over every token in the portfolio, convert it to ETH via the Enso swap target and attempt to send those funds to the caller. However we cannot transfer ETH with ERC20 methods, thus said ETH will be stuck in the batch contract.

The essence of the issue depends on how the Enso swap works with the calldata. The calldata for updating tokens sets the Enso handler as the receiver of the tokens and not the message sender. Thus, the assumption this issue lies on is that the resulting funds from the swap would be in the Batch contract and have to be sent to the user. If the user receives those funds directly, this is invalid

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Recommendation\ Whenever we have specified ETH as the withdraw token, use external calls to the user and not ERC20 transfers.

[langnavina97]

The current implementation is correct. ETH is sent directly to the user, as the receiver is defined in the calldata. Any leftover returned to the user consists of portfolio tokens, which are always ERC20 tokens. @PlamenTSV

[PlamenTSV]

Got it, wasn't sure about the calldata