Uniswap/UniswapX

Fix DirectTakerExecutor ownership issue

marktoda opened this issue · 5 comments

marktoda commented

Right now the DirectTakerExecutor has an issue where anyone can call execute to take earned funds. I think easy solution is to just have configured whitelisted reactor address

marktoda commented

maybe worth considering more general solution - would be nice if single executor could be reactor agnostic but maybe not possible here

marktoda commented

I think #62 is also a great solution for this

azflin commented

Right now the DirectTakerExecutor has an issue where anyone can call execute to take earned funds

How? I thought through this and could not see how this is possible

marktoda commented
  • you are filler
  • you approve executor
  • i am bad guy
  • i call executor.reactorCallback(ResolvedOrder(input: nothing, output: all your tokens to me), taker = you)
  • i call token.safeTransferFrom(executor, me)

I think this works rn, no?

azflin commented

I think this works rn, no?

Yes, I think this works. Had not thought to call the reactorCallback() directly.