EnsoHandler.sol#multiTokenSwapAndTransfer() - swapped tokens would be received by the unintended address

[hats-bug-reporter[bot]]

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

Description: Description\ The Enso handler contract is used to handle swaps at Enso's platform, hardcoded at the SWAP_TARGET via delegatecalls, since Enso's shortcut function require to be delegatecalled. However, depending on how the calldata for the swap is encoded and the logic behind the Enso swapping works, the resulting swapped tokens may end up in the Rebalancer instead of the intended handler contract.

Attack Scenario\ The path I explored with the handler involves updating the token lists:

1. Inside the Rebalancer, whenever we want to invoke updateTokens we update the portfolio list and change their weights
2. _updateWeights is invoked, pulling the tokens out of the vault and swapping them through the Enso handler contract via an external call.
3. We enter Enso's multiTokenSwapAndTransfer for which:
   • the msg.sender is the Rebalancer contract
   • address(this) is the handler itself
4. With the above in mind, this means that the delegatecall SWAP_TARGET.delegatecall(callDataEnso[i]) would retain the same sender and address(this), which means that depending on the Enso swap logic and the calldata provided (probably by the front-end), the swap return may end up in the sender - Rebalancer contract instead of the intended EnsoHandler contract
5. Since the way the contract calculates the swap return is by taking uint256 buyBalanceBefore = IERC20Upgradeable(token).balanceOf(address(this)) and subtracting it from the balance after the swap, but the receiver could be the msg.sender and not address(this), the swapped return tokens would be stuck in the Rebalancer instead.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

Recommendation\

1. When checking the before and after balances, do so by checking the balance of msg.sender, aka the Rebalancer, not the handler itself, since due to the delegatecall, it is not the handler which receives the tokens
2. Change the safe transfer to a safe transferFrom with the correct approval for the returned swap amount, since we need to get the tokens out of the Rebalancer to the _to address and not from the handler.

[langnavina97]

The spender and receiver are defined in the calldata. The tokens are sent to the handler contract, which acts as both the spender and the receiver. If these are incorrectly specified in the calldata, the following check will cause the transaction to revert:

https://github.com/hats-finance/Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77/blob/aa47c9ff85bcc2bede62978c3895668b549da125/contracts/handler/ExternalSwapHandler/EnsoHandler.sol#L71-L72

@PlamenTSV