LACK OF SLIPPAGE PROTECTION IN THE SWAP OPERATION PERFORMED IN THE `Liquidity._dualZapInLiquidity` FUNCTION

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L62

Vulnerability details

Impact

The Liquidity._dualZapInLiquidity function is used to deposit an arbitrary amount of one or both tokens into the pool and receive liquidity corresponding to the value of both of them.

In this function the excessive tokens are swapped such that the liquidity is added to the pool in accordance with the pool reserve ratio. In order to swap the excessive tokens (Eg: swapAmountA to tokenB) the pools.depositSwapWithdraw function is called as shown below:

        zapAmountB += pools.depositSwapWithdraw( tokenA, tokenB, swapAmountA, 0, block.timestamp );

But the issue here is that there is no check on slippage and deadline is too strict. Since the slippage parameter is set to 0 in the function call as shwon above the swap transaction could be executed at a bad price point if the pool is manipulated. This will return less amount of tokenB after the swap. As a result the zapAmountB will be less than expected hence it will not complement the pool reserve ratio when the liquidity is added to the pool.

Furthermore the deadline is set to block.timestamp and this would make it difficult to execute the _dualZapInLiquidity transaction when the blockchain is congested. Since the _dualZapInLiquidity function is expected to be called frequently and the network can be congested this would result in frequent transaction reverts which could lead to bad user experience.

Proof of Concept

            zapAmountB += pools.depositSwapWithdraw( tokenA, tokenB, swapAmountA, 0, block.timestamp );

https://github.com/code-423n4/2024-01-salty/blob/main/src/staking/Liquidity.sol#L62

Tools Used

Manual Review and VSCode

Recommended Mitigation Steps

Hence it is recommended to set a minimum slippage value in the pools.depositSwapWithdraw function call so that bad swap operations can be omitted and set the deadline parameter slightly loosely such that the _dualZapInLiquidity transaction can be executed even during high network congestion without revert.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #805

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid