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

6 stars 4 forks source link

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

Closed sherlock-admin closed 6 months ago

sherlock-admin commented 6 months ago

0xAadi

medium

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


name: Audit item about: These are the audit items that end up in the report title: "" labels: "" assignees: ""

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.

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

  1. 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.

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);
    }

Duplicate of #228