Reentrancy risk of the function _handleSwap()

[shuo-young]

A potential risk lies in the _handleSwap function, which could be reentered by malicious token that implements reentrancy logic to call _handleSwap again. Below shows the possible attack chain.

1. Attacker initializes a malicious pool with registered malicious tokens. A reentancy logic is implemented in the transfer function, which will be invoked in the _handleSwap function (line 281).
2. Bind the malicious pool with the pool manager and the TakeProfitsHook.
3. Attacker prepares the attack by placing order.
4. Attacker begins to swap for the malicious token in the malicious pool to invoke _handleSwap.
5. transfer() function of malicious token is invoked to reenter into the _handleSwap again to swap any tokens of the pools existing in the pool manager (line 275).

There are two main issues lie in the _handleSwap function. (1) There is no access control when performing poolManager.swap() by the parameter key and params, which can be inputted by anyone in this external function. (2) The transfer function of currenry0 could be malicious.

So there maybe some problems when uni v4 enables any one register pools and tokens. To avoid potential vulnerabilities, some protection should added, for example, access control and reentrancyguard modifier. Thanks for your watch and happy to stay in touch.