saucepoint / v4-stoploss

Stop Loss Orders via Uniswap V4 Hooks
MIT License
77 stars 9 forks source link

Possible manipulation by malicious users #8

Open shuo-young opened 6 months ago

shuo-young commented 6 months ago

Dear saucepoint: I find that a potential risk can arise from the possible parameter manipulation of the function afterSwap(). Malicious attackers can input any value of parameters key and params to manipulate intermediate variables and the key stop loss operation in the internal function fillStopLoss() can also be influenced.

I think critical hooks should all be protected which can only be invoked by pool manager. For example, the afterInitialize() is protected in your StopLoss contract. However, the afterSwap() isn't. So adding a simple protective modifier may be good to avoid potential risk.

Hope it helps and glad to discuss further.

saucepoint commented 6 months ago

yep has been flagged before -- just havent had the time to update it. thanks for keeping an eye out!

will leave this open until its fixed 😄