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

6 stars 4 forks source link

kolos3 - The function burn is external wich everyone can call. #248

Closed sherlock-admin closed 7 months ago

sherlock-admin commented 7 months ago

kolos3

high

The function burn is external wich everyone can call.

Summary

The function burn is external wich means everyone can call the function but this is not desired. In the function burn it says "// this low-level function should be called from a contract which performs important safety checks" but this can still be called regardless of the safety checks. An attacker does not have to go through the safety checks he can just call it directly.

Vulnerability Detail

Access controlls

Impact

Everyone can call the function, allowing everyone to burn all the liquidity.

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

Code Snippet


    // this low-level function should be called from a contract which performs important safety checks
    function burn(address to) external lock returns (uint256 amount0, uint256 amount1) {
        (uint112 _reserve0, uint112 _reserve1, ) = getReserves(); // gas savings
        address _token0 = token0; // gas savings
        address _token1 = token1; // gas savings
        uint256 balance0 = IERC20(_token0).balanceOf(address(this));
        uint256 balance1 = IERC20(_token1).balanceOf(address(this));
        uint256 liquidity = balanceOf[address(this)];

        bool feeOn = _mintFee(_reserve0, _reserve1);
        uint256 _totalSupply = totalSupply; // gas savings, must be defined here since totalSupply can update in _mintFee
        amount0 = (liquidity * balance0) / _totalSupply; // using balances ensures pro-rata distribution
        amount1 = (liquidity * balance1) / _totalSupply; // using balances ensures pro-rata distribution
        if (amount0 == 0 || amount1 == 0) revert InsufficientLiquidityBurned();
        _burn(address(this), liquidity);
        _safeTransfer(_token0, to, amount0);
        _safeTransfer(_token1, to, amount1);
        balance0 = IERC20(_token0).balanceOf(address(this));
        balance1 = IERC20(_token1).balanceOf(address(this));

        _update(balance0, balance1, _reserve0, _reserve1);
        if (feeOn) kLast = uint256(reserve0) * reserve1; // reserve0 and reserve1 are up-to-date
        emit Burn(msg.sender, amount0, amount1, to);
    }

Tool used

VsCode, Manual Review

Recommendation

Make the function private.

Duplicate of #95