Fujicracy / fuji-v2

Cross-chain money market aggregator
https://fuji-v2-frontend.vercel.app
15 stars 10 forks source link

Fix Sandwich attack in Router's crosstransfer function #484

Closed pedrovalido closed 1 year ago

pedrovalido commented 1 year ago

This PR addresses H-6

0xcuriousapple commented 1 year ago

hey @DaigaroCota as per as I understand you are accepting delegate as a param now and have replaced it from msg.sender.

if yes can you please explain how it solves the concerned issue? can't the caller pass themselves as delegate only in params?

0xdcota commented 1 year ago

can you please explain how it solves the concerned issue?

The delegate is set by the user-caller who initiated the tx in the origin chain, and they are most likely the receiver of the assets on the destination chain. Previously, as you noted it was the msg.sender. We figured there were situations in where the msg.sender context would be the Connext address; specifically for a three chain operation. Now, delegate as an arg, allows the user to be in control of the slippage all the way through the chain of txns.

can't the caller pass themselves as delegate only in params?

I am failing to find a way how a "malicious" caller, can sandwhich another user. Neither _crossTransfer, or _crossTransferWithCall will allow someone random to pull another user's funds. If the user for some reason sets a random address in control of their "delegate" slippage, I see it no different than approving erc20 to a random address.

0xdcota commented 1 year ago

hey @DaigaroCota as per as I understand you are accepting delegate as a param now and have replaced it from msg.sender.

if yes can you please explain how it solves the concerned issue? can't the caller pass themselves as delegate only in params?

Update was done.