0xAadi - Potential Liquidity Depletion in `Swap` Function Due to AmountOut Exceeding Liquidity Reserves and Resulting Solvency Risks

[sherlock-admin]

0xAadi

medium

Potential Liquidity Depletion in Swap Function Due to AmountOut Exceeding Liquidity Reserves and Resulting Solvency Risks

Summary

Two issues have been identified in the JalaPair contract's swap() function that could lead to liquidity risks:

1. The swap() function permits scenarios where one token's liquidity can be entirely depleted from the pool. This is due to an inadequate conditional check that fails to prevent the liquidity reserves from being completely drained.

2. The function also improperly applies flash loan fees by adjusting the output amounts (amount0Out, amount1Out) after the initial liquidity reserve check. This can result in the reserves being reduced beyond the intended amounts, potentially leading to an underflow in the reserve balances.

Vulnerability Detail

1. The swap() function's conditional check is not robust enough to prevent the total depletion of a token's liquidity from the pool. It allows transactions where the output amount (amount0Out or amount1Out) is equal to the token's reserve, which could lead to a state where the pool has zero liquidity for that token.

File: contracts/JalaPair.sol
213:   if (amount0Out > _reserve0 || amount1Out > _reserve1) revert InsufficientLiquidity();

https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaPair.sol#L213

See line 213, the conditional check fails to revert the transaction when the amountOut is equal to the reserve. This is in contrast to the UniswapV2Pair contract, where a similar check on line 162 ensures that the transaction is reverted if the amountOut is equal to or exceeds the reserve, providing stronger protection against liquidity depletion.

162:   require(amount0Out < _reserve0 && amount1Out < _reserve1, 'UniswapV2: INSUFFICIENT_LIQUIDITY');

https://github.com/Uniswap/v2-core/blob/ee547b17853e71ed4e0101ccfd52e70d5acded58/contracts/UniswapV2Pair.sol#L162

2. The swap() function adjusts the output amounts for flash loan fees after performing the liquidity check, which could lead to the reserves being drawn down more than what was accounted for in the initial check. This can cause the reserves to be lower than expected.

File: contracts/JalaPair.sol

213:  if (amount0Out > _reserve0 || amount1Out > _reserve1) revert InsufficientLiquidity();
.
.
231:  amount0Out = (amount0Out * (10000 + IJalaFactory(factory).flashFee())) / 10000;
.
.
239:  amount1Out = (amount1Out * (10000 + IJalaFactory(factory).flashFee())) / 10000;

https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaPair.sol#L231 https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaPair.sol#L239

Please note that at lines 231 and 239, the amountOut values are updated after the liquidity checks have been performed at line 213. This sequence of operations could lead to discrepancies between the checked and actual amounts withdrawn from the reserves.

Impact

This could effectively disable the pool's ability to facilitate further swaps for the affected token, disrupt market operations, and potentially lead to a loss of the stability and reliability of the liquidity pool. And this issues could disrupt the pricing mechanism of the pool, affect the integrity of transactions, and pose significant financial risks to liquidity providers and users interacting with the pool.

Code Snippet

https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaPair.sol#L213 https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaPair.sol#L231 https://github.com/sherlock-audit/2024-02-jala-swap/blob/030d3ed54214754301154bce0e58ea534100a7e3/jalaswap-dex-contract/contracts/JalaPair.sol#L239

Tool used

Manual Review

Recommendation

Please move the InsufficientLiquidity check after the fee calculation.

    function swap(uint256 amount0Out, uint256 amount1Out, address to, bytes calldata data) external lock {
        if (amount0Out == 0 && amount1Out == 0) revert InsufficientOutputAmount();
        (uint112 _reserve0, uint112 _reserve1, ) = getReserves(); // gas savings
-       if (amount0Out > _reserve0 || amount1Out > _reserve1) revert InsufficientLiquidity();

        uint256 balance0;
        uint256 balance1;
        {
            // scope for _token{0,1}, avoids stack too deep errors
            address _token0 = token0;
            address _token1 = token1;
            if (to == _token0 || to == _token1) revert InvalidTo();
            if (amount0Out > 0) _safeTransfer(_token0, to, amount0Out); // optimistically transfer tokens
            if (amount1Out > 0) _safeTransfer(_token1, to, amount1Out); // optimistically transfer tokens
            if (IJalaFactory(factory).flashOn() && data.length > 0) {
                if (amount0Out > 0) {
                    _safeTransfer(
                        _token0,
                        IJalaFactory(factory).feeTo(),
                        (amount0Out * IJalaFactory(factory).flashFee()) / 10000
                    );
                    amount0Out = (amount0Out * (10000 + IJalaFactory(factory).flashFee())) / 10000;
                }
                if (amount1Out > 0) {
                    _safeTransfer(
                        _token1,
                        IJalaFactory(factory).feeTo(),
                        (amount1Out * IJalaFactory(factory).flashFee()) / 10000
                    );
                    amount1Out = (amount1Out * (10000 + IJalaFactory(factory).flashFee())) / 10000;
                }
                IJalaCallee(to).JalaCall(msg.sender, amount0Out, amount1Out, data);
            }
            balance0 = IERC20(_token0).balanceOf(address(this));
            balance1 = IERC20(_token1).balanceOf(address(this));
        }
+       if (amount0Out >= _reserve0 || amount1Out >= _reserve1) revert InsufficientLiquidity();
        uint256 amount0In = balance0 > _reserve0 - amount0Out ? balance0 - (_reserve0 - amount0Out) : 0;
        uint256 amount1In = balance1 > _reserve1 - amount1Out ? balance1 - (_reserve1 - amount1Out) : 0;
        if (amount0In == 0 && amount1In == 0) revert InsufficientInputAmount();
        {
            // scope for reserve{0,1}Adjusted, avoids stack too deep errors
            uint256 balance0Adjusted = (balance0 * 1000) - (amount0In * 3);
            uint256 balance1Adjusted = (balance1 * 1000) - (amount1In * 3);
            if (balance0Adjusted * balance1Adjusted < uint256(_reserve0) * uint256(_reserve1) * (1000 ** 2))
                revert InvalidK();
        }

        _update(balance0, balance1, _reserve0, _reserve1);
        emit Swap(msg.sender, amount0In, amount1In, amount0Out, amount1Out, to);
    }

[nevillehuang]

request poc

LSW comments:

invalid, cannot bypass the if (balance0Adjusted balance1Adjusted < uint256(_reserve0) uint256(_reserve1) * (1000 ** 2)) check.

[sherlock-admin3]

PoC requested from @AnandkKumaran

Requests remaining: 11

[AnandkKumaran]

This is correct:

Cannot bypass the `if (balance0Adjusted * balance1Adjusted < uint256(_reserve0) * uint256(_reserve1) * (1000 ** 2))` check. 

This issue will not result in any depletion in the reserve. And this issue is invalid.

There is another issue I have noticed while preparing the POC is that if flashFee > 0, this line of code will always be true and revert the execution due to the transfer of flashFee from the JalaPair contract line 226 & 234.

This fee transfer causes an imbalance in the balance and reserve, and the flash loan will be reverted with InvalidK().

I know I am discussing a different issue and this will not count, but the protocol developers can check this issue if they are interested.

POC

Place this test case in JalaPair.t.sol and run forge test --match-path test/JalaPair.t.sol --match-test test_Flashloan

function test_FlashloanSwap() public {
    token0.transfer(address(pair), 1 ether);
    token1.transfer(address(pair), 2 ether);
    pair.mint(address(this));

    uint256 flashloanAmount = 10000;
    uint256 flashloanFee = (flashloanAmount * 1000) / 997 - flashloanAmount + 1;

    Flashloaner fl = new Flashloaner();

    token1.transfer(address(fl), flashloanFee);

    vm.startPrank(feeSetter);
    factory.setFlashOn(true);
    factory.setFeeTo(feeSetter);
    factory.setFlashFee(1 wei);
    vm.stopPrank();

    vm.expectRevert(JalaPair.InvalidK.selector);
    fl.flashloan(address(pair), 0, flashloanAmount, address(token1));
}