M-7 Router slippage checks

[0xdcota]

[M-7] The slippage check on the destination is only done if the first action is deposit/withdrawal

Description

The slippage check is only performed if the first action is a deposit or withdrawal, not for all actions.

ConnextRouter.sol
_accountForSlippage()
if (action == Action.Deposit || action == Action.Payback)

This covers all cases where a user bridges an asset, except in cases where the received funds are used in a nested action.
For example:

1. xReceive with X amount of funds.
   2 .Bundle :
   1. permitWithdraw() 2. withdraw(Y) 3. xCallWithCallData(Deposit (X + Y))

Fuji Team responded to this lead with following :
Response:

In this case after the 3rd action xCallwithCallData, on the destination chain the slippage will be checked on a deposit.

While it is true that slippage would be checked at the final destination, that check would occur for X+ Y in terms of nested xCall slippage parameters, not on X in terms of the initial xCAll. Therefore, the contract would accept any X slippage and execute an xCall to the final destination, missing a required check on the first level.

If X + Y does not fall within the slippage range, the call would revert at the final destination. However, the user's position on the middle chain would already be withdrawn, and they would be unable to take any action to rectify the situation.

Remediation to consider

Consider checking for slippage in xReceive whenever the contract receives any funds.

To avoid looping through all actions to get the amount, consider sending AmountMinOut instead of slippage in calldata.
Note if you decide to solve #455 the hardcoded slippage protections of _crossTransferWithCalldata won’t allow adapting.
by removing the explicit slippage protection, this issue won't be applicable anymore.

[0xdcota]

This issue was addressed with solution proposed in #531 .