recursiveEth - Loss of Funds Due to Inappropriate Handling of Fee Address in Flash Loan Function

[sherlock-admin2]

recursiveEth

high

Loss of Funds Due to Inappropriate Handling of Fee Address in Flash Loan Function

Summary

The smart contract contains a vulnerability that could lead to a loss of funds. In the swap function, if a flash loan is enabled and certain conditions are met, fees are deducted and transferred to an address defined as DEAD, effectively rendering the fees irretrievable. The contract does not appropriately handle the scenario where fees are directed to the DEAD address, leading to the loss of funds.

Vulnerability Detail

In the provided code snippet, the constructor initializes the feeTo variable with the address DEAD. During the execution of the swap function, if the flash loan is enabled and certain conditions are met, fees are deducted and transferred to the feeTo address. Since feeTo is set to DEAD, these fees will effectively be lost and cannot be retrieved

Impact

The impact of this vulnerability is the loss of funds due to the incorrect handling of the fee address. Any fees deducted during the execution of flash loans will be irretrievably sent to the DEAD address, resulting in a permanent loss.

Code Snippet

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

  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();
            // token which went out of the system
            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,
                        //@audit-issue fee is set to DEAD
                        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));
        }
        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);
    }

Tool used

Manual Review

Recommendation

similarly how we check code is checking whether flashLoan is set to true or not they can check whether feeTo() address is set to DEAD or not if yes revert the transaction.

Duplicate of #174