Frontrunning permits

[0xdcota]

[C-4] Frontrunning Permit allows the user to do a different set of actions than the beneficiary intended and, in the worst case run away with funds

Description

The router contract tries to make sure that beneficiary should remain the same, but there is no validation to check if the actions being performed are the ones user intended.

Consider Alice sets withdraw allowance for router through permit and intends to send it to the vault of Chain B.

Anyone can use this Alice permit and do a totally different set of actions. They can xCall and bridge this asset to some random chain.

Alice's Intended Bundle 
1. PermitWithdraw(Owner=Alice, Operator= Router, Receiver= Router)
2. Withdraw (Beneficiary = Alice)
3. xCallWithCalldata(ChainB, Deposit(receiver=alice)) (Beneficiary = Alice)

It is possible to create a different bundle using the same permit.
1. PermitWithdraw(Owner=Alice, Operator= Router, Receiver= Router)
2. Withdraw (Beneficiary = Alice)
3. xCall(any random chain, receiver = Alice) (Beneficiary = Alice)

Fuji Team responded to this lead with following :

Response:

We have referred to these situations as front-running a permit. We believe we have put some measures in place, perhaps not enough.
We ensure that the beneficiary remains the same.
When calling the withdraw action, the beneficiary is set to the owner. Then when attempting to bridge the _beneficiary will be checked and _internalBundle will revert.

It is true that the beneficiary, Alice, in this case, remains the same. However, it is not guaranteed that only Alice's intended actions will be done using her permit. This significantly impacts user experience, which directly interferes with the core intention of the Fuji protocol: bundling user actions through debt permits to enable cross-chain functionality.

Not only is this a user experience issue, but there are also ways attackers can steal from Alice, keeping Beneficiary=Alice only

1. An attacker can manipulate the swap parameters of a swap action since they are user input. They can either add a swap action on top of Alice's bundle or edit it if it's already there.

   Currently, there is no check on the beneficiary as well as explained in #420 However, even if you add check, the exposure to this issue remains.

(
    ISwapper swapper,
    address assetIn,
    address assetOut,
    uint256 amountIn,
    uint256 amountOut,
    address receiver,
    address sweeper,
    uint256 minSweepOut
) = abi.decode(
    args[i],
    (
        ISwapper,
        address,
        address,
        uint256,
        uint256,
        address,
        address,
        uint256
    )
); // @audit one can basically tweak any parameter of this as of now

a. Sandwich Attack

An attacker can allow very high slippage for Alice's action and sandwich their own action in between. They can even change the assetOut address to select the pool with the lowest liquidity.

b. Sweeper Attack

An attacker can set the amountOut to a very low value, set themselves as the sweeper, and run away with leftover amountIn (user funds).

There is no check on who the sweeper could be. With the right parameters, attackers can steal nearly everything.

Please note that in both cases, the beneficiary or receiver can remain the same.

2. Another way is not as easy as the method described above, but the attacker can basically pass any amount of slippage in xCalls and sandwich them on the destination.

To sum up, simply checking the beneficiary is not enough. Many other parameters can impact the amount received by the beneficiary and benefit the attacker.

Remediation

Consider embedding the hash of actions the user intends to perform with their arguments inside the signature. This would resolve many of the issues mentioned in the report.

For example, there would no longer be a need to whitelist swapper, flasher, or vault. Check #420 and #421 .