LI.FI contest - WatchPug's results

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22-L30

Vulnerability details

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22-L30

function swapTokensGeneric(LiFiData memory _lifiData, LibSwap.SwapData[] calldata _swapData) public payable {
    uint256 receivingAssetIdBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId);

    // Swap
    _executeSwaps(_lifiData, _swapData);

    uint256 postSwapBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId) - receivingAssetIdBalance;

    LibAsset.transferAsset(_lifiData.receivingAssetId, payable(msg.sender), postSwapBalance);

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L29-L58

function swap(bytes32 transactionId, SwapData calldata _swapData) internal {
    uint256 fromAmount = _swapData.fromAmount;
    uint256 toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId);
    address fromAssetId = _swapData.sendingAssetId;
    if (!LibAsset.isNativeAsset(fromAssetId) && LibAsset.getOwnBalance(fromAssetId) < fromAmount) {
        LibAsset.transferFromERC20(fromAssetId, msg.sender, address(this), fromAmount);
    }

    if (!LibAsset.isNativeAsset(fromAssetId)) {
        LibAsset.approveERC20(IERC20(fromAssetId), _swapData.approveTo, fromAmount);
    }

    // solhint-disable-next-line avoid-low-level-calls
    (bool success, bytes memory res) = _swapData.callTo.call{ value: msg.value }(_swapData.callData);
    if (!success) {
        string memory reason = LibUtil.getRevertMsg(res);
        revert(reason);
    }

    toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId) - toAmount;
    emit AssetSwapped(
        transactionId,
        _swapData.callTo,
        _swapData.sendingAssetId,
        _swapData.receivingAssetId,
        fromAmount,
        toAmount,
        block.timestamp
    );
}

The Swapper allow arbitrary _swapData (0x style), this makes it possible for a attacker to steal the funds in the contract.

Based on the context, we beleive it's possible that the contract can hold funds.

The funds can be the refunds of failed orders, or fee rebates from bridging or dex aggregators, etc.

See also the permissioned WithdrawFacet.

PoC

Given:

• There are 100 USDC tokens in the contract.

The attacker can submit a swapTokensGeneric() with USDT as receivingAssetId with the following SwapData[]:

• 100 USDC to USDT;

As a result, the attacker will receive ~100 USDT with 0 USDC paid.

Recommendation

Given that Swapper is a standlone module that can be and should be deployed as a standalone contract, we suggest spin it off from the diamond so that it can no longer access the funds in the diamond contract.

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22-L43

Vulnerability details

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/GenericSwapFacet.sol#L22-L43

function swapTokensGeneric(LiFiData memory _lifiData, LibSwap.SwapData[] calldata _swapData) public payable {
    uint256 receivingAssetIdBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId);

    // Swap
    _executeSwaps(_lifiData, _swapData);

    uint256 postSwapBalance = LibAsset.getOwnBalance(_lifiData.receivingAssetId) - receivingAssetIdBalance;

    LibAsset.transferAsset(_lifiData.receivingAssetId, payable(msg.sender), postSwapBalance);

    emit LiFiTransferStarted(
        _lifiData.transactionId,
        _lifiData.integrator,
        _lifiData.referrer,
        _lifiData.sendingAssetId,
        _lifiData.receivingAssetId,
        _lifiData.receiver,
        _lifiData.amount,
        _lifiData.destinationChainId,
        block.timestamp
    );
}

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L12-L22

function _executeSwaps(LiFiData memory _lifiData, LibSwap.SwapData[] calldata _swapData) internal {
    // Swap
    for (uint8 i; i < _swapData.length; i++) {
        require(
            ls.dexWhitelist[_swapData[i].approveTo] == true && ls.dexWhitelist[_swapData[i].callTo] == true,
            "Contract call not allowed!"
        );

        LibSwap.swap(_lifiData.transactionId, _swapData[i]);
    }
}

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L29-L58

function swap(bytes32 transactionId, SwapData calldata _swapData) internal {
    uint256 fromAmount = _swapData.fromAmount;
    uint256 toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId);
    address fromAssetId = _swapData.sendingAssetId;
    if (!LibAsset.isNativeAsset(fromAssetId) && LibAsset.getOwnBalance(fromAssetId) < fromAmount) {
        LibAsset.transferFromERC20(fromAssetId, msg.sender, address(this), fromAmount);
    }

    if (!LibAsset.isNativeAsset(fromAssetId)) {
        LibAsset.approveERC20(IERC20(fromAssetId), _swapData.approveTo, fromAmount);
    }

    // solhint-disable-next-line avoid-low-level-calls
    (bool success, bytes memory res) = _swapData.callTo.call{ value: msg.value }(_swapData.callData);
    if (!success) {
        string memory reason = LibUtil.getRevertMsg(res);
        revert(reason);
    }

    toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId) - toAmount;
    emit AssetSwapped(
        transactionId,
        _swapData.callTo,
        _swapData.sendingAssetId,
        _swapData.receivingAssetId,
        fromAmount,
        toAmount,
        block.timestamp
    );
}

In the AnyswapFacet.sol, _anyswapData.router is from the caller's calldata, which can really be any contract, including a fake Anyswap router contract, as long as it complies to the interfaces used.

And in _startBridge, it will grant infinite approval for the _anyswapData.token to the _anyswapData.router.

This makes it possible for a attacker to steal all the funds from the contract.

Which we explained in [WP-H6], the diamond contract may be holding some funds for various of reasons.

PoC

Given:

• ETH price is 10,000 USDC per ETH;
• There are 0.1 ETH (native tokens) in the contract.

The attacker can submit a swapTokensGeneric() with 0.1 ETH as receivingAssetId with the following SwapData[]: 1. Swap 0.1 ETH to USDC; 2. Swap 0.1 ETH to USDC.

The attacker will receive ~2,000 USDC. 1,000 of which is the 0.1 ETH stolen from the contract.

Recommendation

1. Make sure to only call with {value: msg.value} when LibAsset.isNativeAsset(fromAssetId):

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibSwap.sol#L29-L46

function swap(bytes32 transactionId, SwapData calldata _swapData) internal {
    uint256 fromAmount = _swapData.fromAmount;
    uint256 toAmount = LibAsset.getOwnBalance(_swapData.receivingAssetId);
    address fromAssetId = _swapData.sendingAssetId;
    if (!LibAsset.isNativeAsset(fromAssetId) && LibAsset.getOwnBalance(fromAssetId) < fromAmount) {
        LibAsset.transferFromERC20(fromAssetId, msg.sender, address(this), fromAmount);
    }

    uint256 callValue = msg.value;
    if (!LibAsset.isNativeAsset(fromAssetId)) {
        callValue = 0;
        LibAsset.approveERC20(IERC20(fromAssetId), _swapData.approveTo, fromAmount);
    }

    // solhint-disable-next-line avoid-low-level-calls
    (bool success, bytes memory res) = _swapData.callTo.call{ value: callValue }(_swapData.callData);
    if (!success) {
        string memory reason = LibUtil.getRevertMsg(res);
        revert(reason);
    }

2. Consider spinning off the swapper contract. See also [WP-H6].

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L142-L157

Vulnerability details

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/CBridgeFacet.sol#L142-L157

function _startBridge(CBridgeData memory _cBridgeData) internal {
    Storage storage s = getStorage();
    address bridge = _bridge();

    // Do CBridge stuff
    require(s.cBridgeChainId != _cBridgeData.dstChainId, "Cannot bridge to the same network.");

    if (LibAsset.isNativeAsset(_cBridgeData.token)) {
        ICBridge(bridge).sendNative(
            _cBridgeData.receiver,
            _cBridgeData.amount,
            _cBridgeData.dstChainId,
            _cBridgeData.nonce,
            _cBridgeData.maxSlippage
        );
    } else {
        ...

See: https://github.com/celer-network/sgn-v2-contracts/blob/b0cf02c15e25f66279420e3ff6a8b2fe07404bab/contracts/Bridge.sol#L69-L91

/**
    * @notice Send a cross-chain transfer via the liquidity pool-based bridge using the native token.
    * @param _receiver The address of the receiver.
    * @param _amount The amount of the transfer.
    * @param _dstChainId The destination chain ID.
    * @param _nonce A unique number. Can be timestamp in practice.
    * @param _maxSlippage The max slippage accepted, given as percentage in point (pip). Eg. 5000 means 0.5%.
    * Must be greater than minimalMaxSlippage. Receiver is guaranteed to receive at least (100% - max slippage percentage) * amount or the
    * transfer can be refunded.
    */
function sendNative(
    address _receiver,
    uint256 _amount,
    uint64 _dstChainId,
    uint64 _nonce,
    uint32 _maxSlippage
) external payable nonReentrant whenNotPaused {
    require(msg.value == _amount, "Amount mismatch");
    require(nativeWrap != address(0), "Native wrap not set");
    bytes32 transferId = _send(_receiver, nativeWrap, _amount, _dstChainId, _nonce, _maxSlippage);
    IWETH(nativeWrap).deposit{value: _amount}();
    emit Send(transferId, msg.sender, _receiver, nativeWrap, _amount, _dstChainId, _nonce, _maxSlippage);
}

ICBridge(bridge).sendNative() should be ICBridge(bridge).sendNative{ value: msg.value }(), and the current implementation will simply revert on the cBridge's side with the "Amount mismatch" error.

Recommendation

Change to: ICBridge(bridge).sendNative{ value: msg.value }()

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L62-L77

Vulnerability details

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L62-L77

function batchRemoveDex(address[] calldata _dexs) external {
    LibDiamond.enforceIsContractOwner();

    for (uint256 i; i < _dexs.length; i++) {
        if (s.dexWhitelist[_dexs[i]] == false) {
            continue;
        }
        s.dexWhitelist[_dexs[i]] = false;
        for (uint256 j; j < s.dexs.length; j++) {
            if (s.dexs[j] == _dexs[i]) {
                _removeDex(j);
                return;
            }
        }
    }
}

PoC

Given:

• A, B, C are all whitelisted DEXs;

1. B and C are found to have security issues and they are to be removed from the whitelist.
2. The ContractOwner called batchRemoveDex() to remove B and C.

The transaction was successful. However, only B was removed, C is still on the whitelist.

The users who continues to use C can suffer from fund loss.

Recommendation

Change to:

function batchRemoveDex(address[] calldata _dexs) external {
    LibDiamond.enforceIsContractOwner();

    for (uint256 i; i < _dexs.length; i++) {
        if (s.dexWhitelist[_dexs[i]] == false) {
            continue;
        }
        s.dexWhitelist[_dexs[i]] = false;
        for (uint256 j; j < s.dexs.length; j++) {
            if (s.dexs[j] == _dexs[i]) {
                _removeDex(j);
                break;
            }
        }
    }
}

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L131-L157

Vulnerability details

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L131-L157

function _startBridge(AnyswapData memory _anyswapData) internal {
    // Check chain id
    require(block.chainid != _anyswapData.toChainId, "Cannot bridge to the same network.");
    address underlyingToken = IAnyswapToken(_anyswapData.token).underlying();

    if (underlyingToken == IAnyswapRouter(_anyswapData.router).wNATIVE()) {
        IAnyswapRouter(_anyswapData.router).anySwapOutNative{ value: _anyswapData.amount }(
            _anyswapData.token,
            _anyswapData.recipient,
            _anyswapData.toChainId
        );
        return;
    }

    if (_anyswapData.token != address(0)) {
        // Has underlying token?
        if (underlyingToken != address(0)) {
            // Give Anyswap approval to bridge tokens
            LibAsset.approveERC20(IERC20(underlyingToken), _anyswapData.router, _anyswapData.amount);

            IAnyswapRouter(_anyswapData.router).anySwapOutUnderlying(
                _anyswapData.token,
                _anyswapData.recipient,
                _anyswapData.amount,
                _anyswapData.toChainId
            );
        } else {

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Libraries/LibAsset.sol#L59-L70

function approveERC20(
    IERC20 assetId,
    address spender,
    uint256 amount
) internal {
    if (isNativeAsset(address(assetId))) return;
    uint256 allowance = assetId.allowance(address(this), spender);
    if (allowance < amount) {
        if (allowance > 0) SafeERC20.safeApprove(IERC20(assetId), spender, 0);
        SafeERC20.safeApprove(IERC20(assetId), spender, MAX_INT);
    }
}

In the AnyswapFacet.sol, _anyswapData.router is from the caller's calldata, which can really be any contract, including a fake Anyswap router contract, as long as it complies to the interfaces used.

And in _startBridge, it will grant infinite approval for the _anyswapData.token to the _anyswapData.router.

This makes it possible for a attacker to steal all the funds from the contract.

Which we explained in [WP-H6], the diamond contract may be holding some funds for various of reasons.

PoC

Given:

• There are 100 USDC tokens in the contract.

1. The attacker can submit a startBridgeTokensViaAnyswap() with a FAKE _anyswapData.router.
2. Once the FAKE router contract deployed by the attacker got the infinite approval from the diamond contract, the attacker can call transferFrom() and take all the funds, including the 100 USDC in the contract anytime.

Recommendation

1. Whitelisting the _anyswapData.router rather than trusting user's inputs;
2. Or, only approve() for the amount that required for the current transaction instead of infinite approval.

Lines of code

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/LiFiDiamond.sol#L7-L7

Vulnerability details

Use of Upgradeable Proxy Contract Structure (The Diamond Structure) allows the logic of the contract to be arbitrarily changed.

This allows the proxy admin to perform malicious actions e.g., taking funds from users' wallets up to the allowance limit.

This action can be performed by the malicious/compromised proxy admin without any restriction.

Considering that the main purpose of this particular contract is for bridging tokens with multiple bridge operators, we believe it's not necessary for the diamond contract to hold users' allowances.

PoC

Given:

• token: USDC

Rug Users' Allowances

1. Alice approve() and swapAndStartBridgeTokensViaAnyswap() with 1e8 USDC;
2. Bob approve() and swapAndStartBridgeTokensViaAnyswap() with 5e8 USDC;
3. A malicious/compromised proxy admin can deploy a malicious facet to the diamond contract and stolen all the USDC in Alice and Bob's wallets;

Severity

A smart contract being structured as an upgradeable contract alone is not usually considered as a high severity risk. But given the severe impact (funds in users' wallets got stolen), we mark it as a High severity issue.

Recommendation

Consider using the non-upgradeable Router contract to hold user's allowances instead. Then call transferFrom to pull funds from users' wallets to the the diamond contract and prcess the business login with the diamond contract.

[L] HopFacet#startBridgeTokensViaHop() sendingAssetId with non-native asset, msg.value is not enforced to be 0, makes it possible for users to loss the msg.value if they send any

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L61-L87

    function startBridgeTokensViaHop(LiFiData memory _lifiData, HopData calldata _hopData) public payable {
        address sendingAssetId = _bridge(_hopData.asset).token;

        if (sendingAssetId == address(0)) require(msg.value == _hopData.amount, "ERR_INVALID_AMOUNT");
        else {
            uint256 _sendingAssetIdBalance = LibAsset.getOwnBalance(sendingAssetId);
            LibAsset.transferFromERC20(sendingAssetId, msg.sender, address(this), _hopData.amount);
            require(
                LibAsset.getOwnBalance(sendingAssetId) - _sendingAssetIdBalance == _hopData.amount,
                "ERR_INVALID_AMOUNT"
            );
        }

        _startBridge(_hopData);

        emit LiFiTransferStarted(
            _lifiData.transactionId,
            _lifiData.integrator,
            _lifiData.referrer,
            _lifiData.sendingAssetId,
            _lifiData.receivingAssetId,
            _lifiData.receiver,
            _lifiData.amount,
            _lifiData.destinationChainId,
            block.timestamp
        );
    }

https://github.com/code-423n4/2022-03-lifinance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L134-L178

    function _startBridge(HopData memory _hopData) internal {
        Storage storage s = getStorage();
        address sendingAssetId = _bridge(_hopData.asset).token;

        address bridge;
        if (s.hopChainId == 1) {
            bridge = _bridge(_hopData.asset).bridge;
        } else {
            bridge = _bridge(_hopData.asset).ammWrapper;
        }

        // Do HOP stuff
        require(s.hopChainId != _hopData.chainId, "Cannot bridge to the same network.");

        // Give Hop approval to bridge tokens
        LibAsset.approveERC20(IERC20(sendingAssetId), bridge, _hopData.amount);

        uint256 value = LibAsset.isNativeAsset(address(sendingAssetId)) ? _hopData.amount : 0;

        if (s.hopChainId == 1) {
            // Ethereum L1
            IHopBridge(bridge).sendToL2{ value: value }(
                _hopData.chainId,
                _hopData.recipient,
                _hopData.amount,
                _hopData.destinationAmountOutMin,
                _hopData.destinationDeadline,
                address(0),
                0
            );
        } else {
            // L2
            // solhint-disable-next-line check-send-result
            IHopBridge(bridge).swapAndSend{ value: value }(
                _hopData.chainId,
                _hopData.recipient,
                _hopData.amount,
                _hopData.bonderFee,
                _hopData.amountOutMin,
                _hopData.deadline,
                _hopData.destinationAmountOutMin,
                _hopData.destinationDeadline
            );
        }
    }

Recommendation

Change to:

    function startBridgeTokensViaHop(LiFiData memory _lifiData, HopData calldata _hopData) public payable {
        address sendingAssetId = _bridge(_hopData.asset).token;

        if (sendingAssetId == address(0)) require(msg.value == _hopData.amount, "ERR_INVALID_AMOUNT");
        else {
            require(msg.value == 0, "ERR_INVALID_AMOUNT");
            uint256 _sendingAssetIdBalance = LibAsset.getOwnBalance(sendingAssetId);
            LibAsset.transferFromERC20(sendingAssetId, msg.sender, address(this), _hopData.amount);
            require(
                LibAsset.getOwnBalance(sendingAssetId) - _sendingAssetIdBalance == _hopData.amount,
                "ERR_INVALID_AMOUNT"
            );
        }

        _startBridge(_hopData);

        emit LiFiTransferStarted(
            _lifiData.transactionId,
            _lifiData.integrator,
            _lifiData.referrer,
            _lifiData.sendingAssetId,
            _lifiData.receivingAssetId,
            _lifiData.receiver,
            _lifiData.amount,
            _lifiData.destinationChainId,
            block.timestamp
        );
    }

[N] Use of assert() instead of require()

Contracts use assert() instead of require() in multiple places.

Per to Solidity’s documentation:

"Assert should only be used to test for internal errors, and to check invariants. Properly functioning code should never create a Panic, not even on invalid external input. If this happens, then there is a bug in your contract which you should fix. Language analysis tools can evaluate your contract to identify the conditions and function calls which will cause a Panic.”

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/WithdrawFacet.sol#L30-L30

            assert(_amount <= self.balance);

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/WithdrawFacet.sol#L34-L34

            assert(_amount <= assetBalance);

Recommendation

Change to require().

[N] The same library is being import twice

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L6-L6

import { LibDiamond } from "../Libraries/LibDiamond.sol";

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/AnyswapFacet.sol#L9-L9

import { LibDiamond } from "../Libraries/LibDiamond.sol";

[S]: Suggested optimation, save a decent amount of gas without compromising readability;

[M]: Minor optimation, the amount of gas saved is minor, change when you see fit;

[N]: Non-preferred, the amount of gas saved is at cost of readability, only apply when gas saving is a top priority.

[M] ++i is more efficient than i++

Using ++i is more gas efficient than i++, especially in a loop.

For example:

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L33-L33

        for (uint256 i; i < _dexs.length; i++) {

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L52-L52

        for (uint256 i; i < s.dexs.length; i++) {

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DexManagerFacet.sol#L65-L76

        for (uint256 i; i < _dexs.length; i++) {
            if (s.dexWhitelist[_dexs[i]] == false) {
                continue;
            }
            s.dexWhitelist[_dexs[i]] = false;
            for (uint256 j; j < s.dexs.length; j++) {
                if (s.dexs[j] == _dexs[i]) {
                    _removeDex(j);
                    return;
                }
            }
        }

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/DiamondLoupeFacet.sol#L24-L24

        for (uint256 i; i < numFacets; i++) {

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/HopFacet.sol#L48-L48

        for (uint8 i; i < _tokens.length; i++) {

https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L14-L14

        for (uint8 i; i < _swapData.length; i++) {

[S] Cache array length in for loops can save gas

Reading array length at each iteration of the loop takes 6 gas (3 for mload and 3 to place memory_offset) in the stack.

Caching the array length in the stack saves around 3 gas per iteration.

Instances include:

• https://github.com/code-423n4/2022-03-Li.finance/blob/699c2305fcfb6fe8862b75b26d1d8a2f46a551e6/src/Facets/Swapper.sol#L14-L14

        for (uint8 i; i < _swapData.length; i++)