Lack of slippage control

Summary

Functions JalaMasterRouter.sol::wrapTokensAndaddLiquidity() and JalaMasterRouter.sol::wrapTokenAndaddLiquidityETH() have arguments for slippage control, but they are denoted in unwrapped tokens, which may have 0 decimals, leading to low user's control over preferred slippage level, leading to possible loss of funds.

Vulnerability Detail

Let's look at JalaMasterRouter.sol::wrapTokensAndaddLiquidity() (the second mentioned function has similar problem). amountAMin and amountBMin provided as arguments are denoted in umwrapped tokens. Main idea of master router is that it can be used for tokens that have less than 18 decimals without loss of precision. It was mentioned that "Fan tokens", which are the core of Chilliz' ecosystem, have 0 decimals. This means that the lowest step when providing amountAMin and amountBMin is 1 token when using some of these tokens.

This leads to high slippage which cannot be lowered by the user. For example, if user wants to add X tokens A and 2 tokens B to the pool, for token B they can use either slippage equal to 0%, 50% or 100%. Obviously, zero slippage will likely lead to a revert, so the next best alternative is to use 50% slippage.

Specifically with liquidity adding high slippage is not that vulnerable as it is with swaps, that is why this issue is considered medium.

Though, an example of user's loss is a result of a sandwich attack: 1) user adds liquidity with high slippage for one of the tokens 2) attacker frontruns this call by swapping and changing the ratio of tokens in the pool 3) user adds liquidity with less favourable rate, gets less LP tokens than expected 4) attacker swaps back

This specific example of attack leads to user's small loss of funds, but has high probability and can be frequently repeated. Attacker's profit may be negative in some cases due to 0.3% pool fee, but this can be mitigated if the attacker provides sufficient liquidity to the pool.

Also, it is important to say that frontrun attacks can be executed on Chilliz Chain, as this is a fork of BSC where they can be executed.

Impact

Partial loss of funds when adding liquidity.

Code Snippet

JalaMasterRouter.sol#L32-L69

Tool used

Manual Review

Recommendation

The fix is really simple: use for amountAMin and amountBMin values in wrapped tokens, which will have 18 decimals. This is done in other master router functions - in those that are used for swaps and liquidity removal.

sherlock-audit / 2024-02-jala-swap-judging

pash0k - Lack of slippage control #211