Users may not be able to complete swaps, because arbitrage fails.

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L285-L287 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L243 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L271

Vulnerability details

Impact

Users performing swaps through pools.sol will have their swaps arbitraged by the protocol (if the swap is big enough).

The problem is that users trying to swap tokenA to tokenB through a pool with sufficient liquidity, might have their tx reverted, simply because there might be insufficient liquidity in another pool that is in the arbitrage path. Therefore, users, just trying to swap from tokenA to tokenB will suffer from DOS.

Proof of Concept

1. Assume there's enough liquidity in the pool (tokenA;tokenB) to fulfill the swap. User tries to swap tokenA to tokenB. https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L366

2. Protocol then tries to arbitrage: Assume arbitrage path of weth->tokenB->tokenA->weth

If there isn't enough liquidity before the swap or after the swap in any of the pools, (weth;tokenB) or (weth;tokenA), the swap will not go through (it reverts: https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L243, https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/Pools.sol#L271), which doesn't let the user perform the swap.

The swap would normally go through, because there's enough liquidity to fulfill the order, however, it fails because of insufficient liquidity in some other pool in the arbitrage path.

3. The transaction doesn't go through, which means the user can't perform swaps.

This means the user experiences denial of service.

NOTE: https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/arbitrage/ArbitrageSearch.sol#L101 doesn't fully check whether there's enough liquidity, it can still fail because it doesn't account that the liquidity left has to be more then dust amount.

Tools Used

manual review

Recommended Mitigation Steps

Don't revert while checking if arbitrage is possible. Ideally, code should check whether arbitrage is possible, if it is, make state changes, and if it isn't, don't make any state changes, just perform the user swap and don't revert.

Assessed type

Context

[c4-judge]

Picodes marked the issue as primary issue

[c4-judge]

Picodes marked the issue as duplicate of #245

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid