No slippage limit set in Uniswap swap
Description
When performing swaps to pay for gas with ZRC-20s, the swap is executed with no minimum out parameter set. This can be seen in the invocation of the CallEvm
function with the parameter AmountInMax
set to BigIntZero
.
func (k *Keeper) CallUniswapV2RouterSwapExactTokensForTokens(
ctx sdk.Context,
sender ethcommon.Address,
to ethcommon.Address,
amountIn *big.Int,
inZRC4,
outZRC4 ethcommon.Address,
noEthereumTxEvent bool,
) (ret []*big.Int, err error) {
...
res, err := k.CallEVM(
ctx,
*routerABI,
sender,
routerAddress,
BigIntZero,
big.NewInt(1000_000),
true,
noEthereumTxEvent,
"swapExactTokensForTokens",
amountIn,
BigIntZero,
[]ethcommon.Address{inZRC4, wzetaAddr, outZRC4},
to,
big.NewInt(1e17),
)
...
}
Impact
Users could be subject to sandwich attacks and as a result take on unfavorable prices, resulting in a loss of funds.
Recommendations
Add an external parameter that a user can specify amountOutMin
to prevent such attacks.
Remediation
The ZetaChain team stated that:
We acknowledged the finding as an issue as it might cause the user to pay more fees than what needed for the actual gas of the transaction. However, we decided to not apply the recommendation, setting a AmountOutMin . The reason is if the swap of tokens fails during the gas payment for the revert transaction of the CCTX, the CCTX will become aborted. Aborting the CCTX would cause to refund the user on ZetaChain with the remaining amount. Although it prevents the slippage loss, we deemed it as a worse UX since the user would need to trigger a transaction to get the funds back on the source chain.
and that:
We decided to keep the recommendation in our backlog for implementation in the future. As a remediation we plan to implement off-chain monitoring in order to check the liquidity Uniswap pools for gas payment.